Lb/update languages menu ux behind flag#3335
Conversation
0b1dc58 to
a743b3c
Compare
| } | ||
| } | ||
|
|
||
| nav.m-top-tier-menu--mobile { |
There was a problem hiding this comment.
Tried to follow naming conventions with the new styling classes I added, but I'm sure I'm missing context/it's possible I'm not using them correctly. Please let me know!
There was a problem hiding this comment.
In the Rider Design System, we refer to this as the "MBTA.com Header" (catchy, I know) and the second tier as the "MBTA.com Navbar" component.
| } | ||
| } | ||
|
|
||
| .top-tier-nav-container { |
There was a problem hiding this comment.
Tried to follow naming conventions with the new styling classes I added, but I'm sure I'm missing context/it's possible I'm not using them correctly. Please let me know!
dba6265 to
f9ceaec
Compare
|
|
||
| const header: HTMLElement = rootElement.querySelector("header")!; | ||
| if (!header) return; | ||
| // SUPPORT MULTIPLE HEADERS |
There was a problem hiding this comment.
I know there's been some performance improvement work recently -- this code for handling/looping through multiple headers could benefit from additional eyes on it!
Minor cleanup WIP mobile UX WIP fix up issues with menu open/close Tweak styling Fix a class mistake Update .po files WIP refactors Quiet the TS errors WIP WIP Move languages menu closer to button on mobile Styling tweaks More styling tweaks Move classe to right file WIP clean up styles Some cleanup Add TODO Remove commented-out code
Manually resolve merge conflict Remove unnecessary comments Only make necessary changes Bring code back in that got deleted by accident Restore previous behaviors More styling tweaks
f9ceaec to
f982f18
Compare
thecristen
left a comment
There was a problem hiding this comment.
Thank you for this addition and light refactor - you're a great pattern-matcher!
In addition to the suggestions here, I noticed that the top tier nav completely disappears when you're between certain desktop and mobile breakpoints 😅 (shrink the page width and you'll see!).
| height: $header-navbar-height-new; | ||
|
|
||
| @include media-breakpoint-only(xs) { | ||
| height: $header-navbar-height-xs; | ||
| } |
There was a problem hiding this comment.
suggestion: to better match the design, let's remove the defined height here and add vertical padding! Here's the CSS variable we have which corresponds to the --spacing-small variable used in Figma
| height: $header-navbar-height-new; | |
| @include media-breakpoint-only(xs) { | |
| height: $header-navbar-height-xs; | |
| } | |
| padding: var(--spacing-sm); |
(and both mobile and desktop designs happen to use the same padding)
| @@ -0,0 +1,47 @@ | |||
| <header class="top-tier-header new"> | |||
| <div class="container top-tier-nav-container"> | |||
| <nav id="languages-menu" class="top-tier-header-navbar new m-menu--desktop"> | |||
| @@ -0,0 +1,46 @@ | |||
| <header class="top-tier-header new top-tier-nav"> | |||
| <div class="container top-tier-nav-container new"> | |||
| <nav id="languages-menu" class="top-tier-header-navbar new m-top-tier-menu--mobile"> | |||
There was a problem hiding this comment.
That's a good question...I'll follow up with Chase!
|
|
||
| .top-tier-header { | ||
| .navbar-logo, | ||
| .navbar-logo:hover, | ||
| .navbar-logo:focus { | ||
| color: $white; | ||
| text-decoration: none; | ||
| } | ||
|
|
||
| .c-svg__mbta-logo, | ||
| .c-svg__mbta-name-and-logo { | ||
| path, | ||
| text { | ||
| fill: $white; | ||
| } | ||
| } | ||
|
|
||
| background-color: $brand-primary-darkest; | ||
| color: $white; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
I'm not sure this section is needed, because I think the existing CSS will achieve the same as-is?
There was a problem hiding this comment.
@thecristen There are subtle differences: the logos need to be white, and the background color needs to be darker! I agree there's a lot of repetition here, though. Thoughts on a better way to handle it?
| <%= if not Laboratory.enabled?(@conn, :use_smartling_translations) do %> | ||
| <button | ||
| class="header-search__toggle" | ||
| data-nav="toggle-nav-search" | ||
| type="button" | ||
| aria-expanded="false" | ||
| aria-controls="search-header-mobile__container" | ||
| aria-label={~t(Search MBTA.com)} | ||
| > | ||
| <i class="fa fa-fw fa-search" aria-hidden="true"></i> | ||
| <i class="fa fa-fw fa-close" aria-hidden="true"></i> | ||
| </button> | ||
| <% else %> | ||
| <div class="header-search__toggle__container"> | ||
| <button | ||
| class="header-search__toggle header-search__toggle_with_top_tier_nav" | ||
| data-nav="toggle-nav-search" | ||
| type="button" | ||
| aria-expanded="false" | ||
| aria-controls="search-header-mobile__container" | ||
| aria-label={~t(Search MBTA.com)} | ||
| > | ||
| <i class="fa fa-fw fa-search" aria-hidden="true"></i> | ||
| <i class="fa fa-fw fa-close" aria-hidden="true"></i> | ||
| </button> | ||
| </div> | ||
| <% end %> |
There was a problem hiding this comment.
I'm a bit confused why this diff is needed! Could you explain what's happening here?
There was a problem hiding this comment.
It's a very subtle difference in styling (with the addition of the new classes) that moves the logo in the original header. In the original header, on mobile, the logo is between the menu and the search bar. In the new design, the logo is always left-aligned.
I'm definitely not saying this is the best way to implement that, though -- especially being an Elixir newbie. Let me know your thoughts!
| </a> | ||
|
|
||
| <div class="m-menu--desktop__menu"> | ||
| <div class={"#{if Laboratory.enabled?(@conn, :use_smartling_translations), do: "m-menu--multiple_tiered_desktop__menu", else: "m-menu--desktop__menu"}"}> |
There was a problem hiding this comment.
I think you might be able to get away without toggling the CSS class here (and I don't think you need the .m-menu--multiple_tiered_desktop__menu class either)
There was a problem hiding this comment.
The only important difference included in the new class is this:
top: $header-navbar-height-new * 2;
I'll try to find a better way to account for that without changing the original class!
| @@ -0,0 +1,46 @@ | |||
| <header class="top-tier-header new top-tier-nav"> | |||
There was a problem hiding this comment.
question: Why have a separate template for the mobile version of the top tier nav? There's almost no designed content difference between the desktop/mobile designs, only the MBTA logo change that could be handled in CSS. I think you don't need this file (and the associated CSS duplicated for it).
There was a problem hiding this comment.
There are some different classes applied in either case -- but I'm still getting familiar with Elixir templates, so I imagine there's a better way to handle even that. Any suggestions?


Scope
We want to redesign the header with a two-tier menu that, among other things, reimagines what the Languages button/menu looks like. This adds that redesign behind the feature flag that also enables Smartling translations .
Asana Ticket:
https://app.asana.com/1/15492006741476/project/1213994587940653/task/1216081563934501?focus=true
Implementation
Screenshots
How to test
/_flagsand turn on the feature flagNote: Changes to the footer will come in future work, once designs are complete.