Fixed globe icon placement in buttons by adding left padding #9810

imajit · 2021-06-15T20:37:09Z

Part of #9686
In my last PR I had by reduced the button to the red part in the nav bar

I have corrected that part now the entire button is clickable in cases where globe icon is not there , here entire green part is now clickable

For buttons I have added some left padding in the function,now the icons don't render inside the buttons and in cases where the button occupies the entire width globe icon is rendered below the button
Admin Page Buttons

Dashboard Buttons

Subscriptions Page Buttons

The add button here is a submit button and has some tests linked with it. If you feel it is okay with the nested globe icon I can just leave it unchanged, else I'll change it with the globe icon outside the button and change the tests for it in a later PR.

gitpod-io · 2021-06-15T20:37:11Z

codecov · 2021-06-15T20:44:36Z

Codecov Report

❗ No coverage uploaded for pull request base (main@cf6f681). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head d12a468 differs from pull request most recent head d127d0d. Consider uploading reports for the commit d127d0d to get more accurate results

@@           Coverage Diff           @@
##             main    #9810   +/-   ##
=======================================
  Coverage        ?   78.24%           
=======================================
  Files           ?       98           
  Lines           ?     5947           
  Branches        ?        0           
=======================================
  Hits            ?     4653           
  Misses          ?     1294           
  Partials        ?        0

jywarren · 2021-06-15T22:52:40Z

Hi @imajit this looks lovely! Thank you for your attention to detail as well!

I think we used to limit this to 5 max appearing on the screen so that it's not overwhelming. Did we ever implement that? I think this is fine but that's the only thing I worry a little about here. Do you perhaps want to address that soon too? I don't think it has to block merging this one.

Thank you!!!

jywarren · 2021-06-15T22:54:59Z

app/views/sidebar/_dashboard.html.erb

-        <a class="dropdown-item" href="/post?template=event&tags=event"><%= translation('dashboard._header.dropdown.post_event') %></a>
-        <div class="dropdown-divider"></div>
-        <a class="dropdown-item" href="/post?tags=blog-submission"><%= translation('dashboard._header.dropdown.tell_story_blog') %></a>
+        <% if current_user&.has_tag('translation-helper') && I18n.locale != :en %>


Ooh, actually... this is interesting. This repetition does make for a lot more code... i wonder... what do you think, is there any way to reduce repetition in this while still achieving what we want? What are some ideas to try to brainstorm this?

Sorry to pause a moment, but I hope my hesitation makes sense, @imajit -- still very much appreciate this work!

Yeah, I agree there is a repetition in the code as I'm wrapping the button inside a div so that the globe icon stays inline with the text.

If I keep the if part only, then the button size reduces for all users

If I keep the else part, then the globe icon overflows to the next line

I can't store it in a new file as partial as the repetition is kind of nested inside each div
So, I did this bit ugly implementation to satisfy all cases. One way to avoid this is if I reduce the number of globe icons in the navbar, in that case, there will be less code repetition and fewer number of globe icons on the Dashboard.
For now, I can't see a different way of implementation though 😕

Hmm.... I totally appreciate this is a tough one. Thanks for thinking hard on it. What if... we did it in JavaScript, after rendering? Could that work?

I added a helper function to render the dropdown button, this reduces some repetition

imajit · 2021-06-16T11:27:53Z

Hi @imajit this looks lovely! Thank you for your attention to detail as well!

I think we used to limit this to 5 max appearing on the screen so that it's not overwhelming. Did we ever implement that? I think this is fine but that's the only thing I worry a little about here. Do you perhaps want to address that soon too? I don't think it has to block merging this one.

Thank you!!!

I did read about it in a discussion #5651 (comment) but I don't think it has been implemented, there is a way using jquery to do this,
jQuery(".translationClass").children(":gt(6)").hide();
this will show only 5 globe icons, but I have never tested it on a live server with multiple users.

jywarren · 2021-06-17T22:08:56Z

this will show only 5 globe icons, but I have never tested it on a live server with multiple users.

Did you want to try it in the JavaScript dev console on stable.publiclab.org to get a sense for it?

codeclimate · 2021-06-19T11:29:57Z

app/helpers/application_helper.rb

          <i data-toggle='tooltip' data-placement='top' title='Needs translation? Click to help translate this text.' style='position:relative; right:2px; color:#bbb; font-size: 15px;' class='fa fa-globe'></i></a>
       </span>))
    else
      raw(translated_string)
    end
  end
+
+  def create_dropdown(href,text)


Space missing after comma.

codeclimate · 2021-06-19T11:29:57Z

app/helpers/application_helper.rb

          <i data-toggle='tooltip' data-placement='top' title='Needs translation? Click to help translate this text.' style='position:relative; right:2px; color:#bbb; font-size: 15px;' class='fa fa-globe'></i></a>
       </span>))
    else
      raw(translated_string)
    end
  end
+
+  def create_dropdown(href,text)
+    translated_string=translation(text)


Surrounding space missing for operator =.

jywarren

The helper function does look good. I suggested a slight renaming. Does this approach work for the lines in this partial too, like lines 11-26?

jywarren · 2021-06-21T22:11:04Z

app/helpers/application_helper.rb

          <i data-toggle='tooltip' data-placement='top' title='Needs translation? Click to help translate this text.' style='position:relative; right:2px; color:#bbb; font-size: 15px;' class='fa fa-globe'></i></a>
       </span>))
    else
      raw(translated_string)
    end
  end
+
+  def create_dropdown(href, text)


Ah, this is much cleaner. Could we rename this though so it's a little more specific? Let me think ---

Suggested change

def create_dropdown(href, text)

def create_nav_dropdown_item(href, text)

OK awesome. I think this is one of the last things before we merge, right? Thank you!!

jywarren · 2021-06-21T22:12:59Z

Also, this will need conflicts resolve as well. Thanks for sticking with this @imajit -- much appreciated!

imajit · 2021-06-22T11:33:53Z

Hmm, I think this is due to some recently added tests , looking into it

* Reduced globe icon count * Added equal sign Co-authored-by: imajit <ajit.171it233@nitk.edu.in>

jywarren · 2021-06-22T20:03:36Z

Ah indeed. You can try resolving directly or rebasing!

* 🐛 minor bug fix, addressing issue with first timers adding tags. * ♻️ minor refactor, using function call to compare user role. * ♻️🔥 remove logged_in_as function call in conditional statement. * fixing failing tests by blocking first time posters from adding tags Co-authored-by: 17sushmita <17sushmita@gmail.com>

codeclimate · 2021-06-23T18:41:19Z

Code Climate has analyzed commit d127d0d and detected 0 issues on this pull request.

View more on Code Climate.

jywarren · 2021-06-29T16:58:41Z

Nice! Thanks for doing this so thoroughly, @imajit !!

…ab#9810) * Dropdown fixed * Fixed button 1 * Button fix 2 * Added helper function to render dropdown buttons * added space * Changed function name * Reduced globe icon count (publiclab#9826) * Reduced globe icon count * Added equal sign Co-authored-by: imajit <ajit.171it233@nitk.edu.in> * Update questionRich.html.erb (publiclab#9841) * Addressing issue with first timers adding tags. (publiclab#9829) * 🐛 minor bug fix, addressing issue with first timers adding tags. * ♻️ minor refactor, using function call to compare user role. * ♻️🔥 remove logged_in_as function call in conditional statement. * fixing failing tests by blocking first time posters from adding tags Co-authored-by: 17sushmita <17sushmita@gmail.com> * Dropdown fixed * Fixed button 1 * Button fix 2 * Added helper function to render dropdown buttons * added space * Changed function name Co-authored-by: imajit <ajit.171it233@nitk.edu.in> Co-authored-by: Jeffrey Warren <jeff@unterbahn.com> Co-authored-by: lonwabo <60598841+lonwabo-mnyaiza@users.noreply.github.com> Co-authored-by: 17sushmita <17sushmita@gmail.com>

imajit added 3 commits June 15, 2021 16:37

Dropdown fixed

3175032

Fixed button 1

8b3638f

Button fix 2

5702a22

imajit requested a review from a team as a code owner June 15, 2021 20:37

jywarren reviewed Jun 15, 2021

View reviewed changes

Added helper function to render dropdown buttons

1aaf43b

codeclimate bot reviewed Jun 19, 2021

View reviewed changes

added space

ec281d4

jywarren requested changes Jun 21, 2021

View reviewed changes

imajit and others added 2 commits June 22, 2021 16:01

Changed function name

d306dee

Merge branch 'main' into button-fix

4f2e537

Reduced globe icon count (publiclab#9826)

b3dab52

* Reduced globe icon count * Added equal sign Co-authored-by: imajit <ajit.171it233@nitk.edu.in>

jywarren and others added 9 commits June 22, 2021 18:14

Update questionRich.html.erb (publiclab#9841)

3a6c594

Dropdown fixed

20ac7f1

Fixed button 1

bbd033c

Button fix 2

cdd9f81

Added helper function to render dropdown buttons

c58929f

added space

179af43

Changed function name

f346fea

Fixed merge conflicts

d127d0d

imajit requested a review from a team as a code owner June 23, 2021 18:40

jywarren approved these changes Jun 29, 2021

View reviewed changes

jywarren merged commit d092648 into publiclab:main Jun 29, 2021

imajit deleted the button-fix branch June 30, 2021 13:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed globe icon placement in buttons by adding left padding #9810

Fixed globe icon placement in buttons by adding left padding #9810

imajit commented Jun 15, 2021

gitpod-io bot commented Jun 15, 2021

codecov bot commented Jun 15, 2021 •

edited

Loading

jywarren commented Jun 15, 2021

jywarren Jun 15, 2021

imajit Jun 16, 2021

jywarren Jun 17, 2021

imajit Jun 19, 2021

imajit commented Jun 16, 2021

jywarren commented Jun 17, 2021

codeclimate bot Jun 19, 2021

codeclimate bot Jun 19, 2021

jywarren left a comment

jywarren Jun 21, 2021

jywarren Jun 23, 2021

imajit Jun 23, 2021

jywarren commented Jun 21, 2021

imajit commented Jun 22, 2021 •

edited

Loading

jywarren commented Jun 22, 2021

codeclimate bot commented Jun 23, 2021

jywarren commented Jun 29, 2021

	def create_dropdown(href, text)
	def create_nav_dropdown_item(href, text)

Fixed globe icon placement in buttons by adding left padding #9810

Fixed globe icon placement in buttons by adding left padding #9810

Conversation

imajit commented Jun 15, 2021

gitpod-io bot commented Jun 15, 2021

codecov bot commented Jun 15, 2021 • edited Loading

Codecov Report

jywarren commented Jun 15, 2021

jywarren Jun 15, 2021

Choose a reason for hiding this comment

imajit Jun 16, 2021

Choose a reason for hiding this comment

jywarren Jun 17, 2021

Choose a reason for hiding this comment

imajit Jun 19, 2021

Choose a reason for hiding this comment

imajit commented Jun 16, 2021

jywarren commented Jun 17, 2021

codeclimate bot Jun 19, 2021

Choose a reason for hiding this comment

codeclimate bot Jun 19, 2021

Choose a reason for hiding this comment

jywarren left a comment

Choose a reason for hiding this comment

jywarren Jun 21, 2021

Choose a reason for hiding this comment

jywarren Jun 23, 2021

Choose a reason for hiding this comment

imajit Jun 23, 2021

Choose a reason for hiding this comment

jywarren commented Jun 21, 2021

imajit commented Jun 22, 2021 • edited Loading

jywarren commented Jun 22, 2021

codeclimate bot commented Jun 23, 2021

jywarren commented Jun 29, 2021

codecov bot commented Jun 15, 2021 •

edited

Loading

imajit commented Jun 22, 2021 •

edited

Loading