USWDS - Header: Vertically center icon in nav #5654
Conversation
Styles were largely duplicated across
…ical-center-nav-expander
USWDS - Nav: Vertically center nav expander button
…lly-center-nav-elements
amyleadem
changed the title
| &[aria-expanded] { | ||
| span { | ||
| @include place-icon($-add-icon, "after"); | ||
|
|
||
| &::after { | ||
| position: absolute; | ||
| top: 50%; | ||
| right: 0; | ||
| transform: translateY(-50%); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
[aria-expanded] vs accordion-button
Im curious if it would make more sense if we moved these lines to usa-nav__primary.usa-accordion__button.span on line 300 instead of [aria-expanded]. Seems like it might be a bit more descriptive but it is further away from the other aria-expanded=true/false styles which are relevant to this work.
uswds/packages/usa-nav/src/styles/_usa-nav.scss
Lines 307 to 315 in 05e4ee2
There was a problem hiding this comment.
In future work this type of research should be part of the initial stages. This will help us write more maintainable code and robust features.
There was a problem hiding this comment.
I agree with James's point. We don't need to change this work, but tying our styles to ARIA roles is something that may not stand the test of time
There was a problem hiding this comment.
This is looking good to me. I tested the following in Safari, Chrome, and Firefox:
- Confirm icon is vertically centered in mobile with two lines of text
- Confirm icon is vertically centered in desktop with two lines of text
- Note: There are still some quirks when the navbar elements have different text heights. (Screenshot below). There is also an issue with the text overlapping the icon in desktop view. However, these are both pre-existing issues in develop. The issue has been captured in issue USWDS - Header: Line breaks cause text to overflow into padding area #5656 and a fix is in progress in PR USWDS - Nav: Resolve padding overflow bug #5655. Since this is a pre-existing issue, I don't think it should block anything.
- Note: There are still some quirks when the navbar elements have different text heights. (Screenshot below). There is also an issue with the text overlapping the icon in desktop view. However, these are both pre-existing issues in develop. The issue has been captured in issue USWDS - Header: Line breaks cause text to overflow into padding area #5656 and a fix is in progress in PR USWDS - Nav: Resolve padding overflow bug #5655. Since this is a pre-existing issue, I don't think it should block anything.
- Confirm icon is vertically centered in mobile with one line of text
- Confirm icon is vertically centered in desktop with one line of text
- Confirm icon is correctly aligned when item is expanded
- Confirm no regressions in hover state colors in both desktop and mobile
- Confirm no regressions in high contrast mode in both desktop and mobile
- Confirmed for all variants of the header
mejiaj
changed the title
Summary
Fixed the vertical alignment of the expand icon on header navigation items.
Related issue
Closes #5660
Preview link
Header component
Related PR
This work originated in #5267 (Thanks @aduth!)
Changelog →
Problem statement
When navigation links include text which breaks to multiple lines (particularly in mobile navigation), the expander icon is shown at the bottom right of the link, which makes it more difficult to identify.
Solution
Vertically center the icon.
Testing and review
Testing checklist