New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make items in user dropdown menu easier to click. #1635

Closed
flukeout opened this Issue Jan 16, 2017 · 6 comments

Comments

Projects
3 participants
@flukeout
Contributor

flukeout commented Jan 16, 2017

The items in the dropdown menus only work if you click the text of the item, and not the entire length and height of the highlighted item when you hover.

This is because each item is a...

<li>
  <a>Item Label</a>
</li>

...and the padding around the item is applied to the <li> which isn't clickable.

image

To fix these we either...

  • Add the click listener to the <li> item, or..
  • Change the CSS styling so that the <a> item gets the padding and then makes up the entire clickable area.
@flukeout

This comment has been minimized.

Show comment
Hide comment
@flukeout

flukeout Jan 16, 2017

Contributor

This is probably the same issue that is causing #1534

Contributor

flukeout commented Jan 16, 2017

This is probably the same issue that is causing #1534

@flukeout flukeout changed the title from Items in dropdown menu are not easy to click. to Make items in user dropdown menu easier to click. Jan 16, 2017

@raygervais

This comment has been minimized.

Show comment
Hide comment
@raygervais

raygervais Jan 27, 2017

Contributor

@flukeout @humphd, I'd like to fix this bug. Can you point me to the coding conventions & contributions document?

Contributor

raygervais commented Jan 27, 2017

@flukeout @humphd, I'd like to fix this bug. Can you point me to the coding conventions & contributions document?

@flukeout

This comment has been minimized.

Show comment
Hide comment
@flukeout

flukeout Jan 27, 2017

Contributor

@raygervais I'm not sure we have a coding conventions doc. Do we @gideonthomas

As far as contributing, the first step would be to get your dev environment up and running. Follow the steps in the readme here... https://github.com/mozilla/thimble.mozilla.org

Then, you're free to get rolling and ask us for help if you get stuck.

Contributor

flukeout commented Jan 27, 2017

@raygervais I'm not sure we have a coding conventions doc. Do we @gideonthomas

As far as contributing, the first step would be to get your dev environment up and running. Follow the steps in the readme here... https://github.com/mozilla/thimble.mozilla.org

Then, you're free to get rolling and ask us for help if you get stuck.

@gideonthomas

This comment has been minimized.

Show comment
Hide comment
@gideonthomas

gideonthomas Feb 1, 2017

Member

No coding conventions document yet since we don't specifically follow a style guide. @raygervais, for now, I would try to follow the conventions that you see in the file you are working with. If there are any issues with coding conventions, we will point them out in your PR so I wouldn't worry about them at the moment.

Member

gideonthomas commented Feb 1, 2017

No coding conventions document yet since we don't specifically follow a style guide. @raygervais, for now, I would try to follow the conventions that you see in the file you are working with. If there are any issues with coding conventions, we will point them out in your PR so I wouldn't worry about them at the moment.

@raygervais

This comment has been minimized.

Show comment
Hide comment
@raygervais

raygervais Feb 3, 2017

Contributor

@gideonthomas thank you for the advice. @flukeout, I've attached an example of your second suggested option to fix this issue, and believe it is the most optimal way as well. You'll see that I used Firefox's highlight tool to ensure that it would not affect any other element on the page. In doing so, I also reduced surrounding <li> element's padding to 0. I also added display:block to retain the original styling of the element.
css_thimble_fix_1635

If you are satisfied with this, I'd be happy to start a PR right away. Furthermore, the height and width made no difference here, so I will not include in the final commit.

Contributor

raygervais commented Feb 3, 2017

@gideonthomas thank you for the advice. @flukeout, I've attached an example of your second suggested option to fix this issue, and believe it is the most optimal way as well. You'll see that I used Firefox's highlight tool to ensure that it would not affect any other element on the page. In doing so, I also reduced surrounding <li> element's padding to 0. I also added display:block to retain the original styling of the element.
css_thimble_fix_1635

If you are satisfied with this, I'd be happy to start a PR right away. Furthermore, the height and width made no difference here, so I will not include in the final commit.

@flukeout

This comment has been minimized.

Show comment
Hide comment
@flukeout

flukeout Feb 3, 2017

Contributor

Overall, this is great. Here are some thoughts.

You can remove the height from the li element if it has one, and just use the padding in the a element to give the menu item its height.

That way you can drop the height declaration from the a. Additionally, block elements always span 100% of the width of their parent. So you don't need to set a width on it either.

Give that a try and please go ahead with starting the PR. Looking forward.

Contributor

flukeout commented Feb 3, 2017

Overall, this is great. Here are some thoughts.

You can remove the height from the li element if it has one, and just use the padding in the a element to give the menu item its height.

That way you can drop the height declaration from the a. Additionally, block elements always span 100% of the width of their parent. So you don't need to set a width on it either.

Give that a try and please go ahead with starting the PR. Looking forward.

@gideonthomas gideonthomas added this to Assigned in Technical Debt Feb 3, 2017

@gideonthomas gideonthomas moved this from Assigned to Needs Review in Technical Debt Feb 3, 2017

flukeout pushed a commit that referenced this issue Feb 7, 2017

Fix #1635 - Fixed logged in user dropdown menu easier to click (#1697)
* Fix #1635 - Fixed loggedin user dropdown menu easier to click

* Update userbar.less

Removed 'px' from line 480 to satisfy coding standards.

* Update userbar.less

Removed the display:block property from <li> elements in the dropdown-content namespace.

@gideonthomas gideonthomas moved this from Needs Review to Closed in Technical Debt Feb 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment