Lb/feature flag with new language picker#3289
Conversation
lilybarrett
force-pushed
the
lb/feature-flag-with-new-language-picker
branch
from
aab068c to
406d9cf
Compare
…ratory flag WIP Play around with locale testing Some tweaks WIP barebones language picker for Smartling WIP ne Languages picker on feature flag
Tweak for menu
lilybarrett
force-pushed
the
lb/feature-flag-with-new-language-picker
branch
from
033afdc to
d7d64ee
Compare
lvachon1
left a comment
lvachon1
left a comment
There was a problem hiding this comment.
This works great, I'm excited to see this whole translation project get properly underway here on MBTA.com
joshlarson
left a comment
joshlarson
left a comment
There was a problem hiding this comment.
I see that @lvachon1 approved (thank you Luc!). I was snooping a bit, and had a few non-blocking questions and thoughts, which I may as well still leave here.
Some of them may need to be addressed before this becomes fully productionalized, but I don't think any of them need to block further development on this feature (and possibly some or all of the issues/questions are really more core-dotcom responsibilities anyway, and don't need to take up your time! 😅)
Thanks for this work!
| <% end %> | ||
| <div class="body-wrapper" id="body-wrapper"> | ||
| <%= if !Application.get_env(:dotcom, :is_prod_env?) do %> | ||
| <%= if !Application.get_env(:dotcom, :is_prod_env?) && !Laboratory.enabled?(@conn, :use_smartling_translations) do %> |
There was a problem hiding this comment.
Question/Request (non-blocking): Would it be possible to keep that banner at the top when Smartling is enabled? Or was the wacky "menu location is actually hard-coded, and gets messed up by the banner" issue too annoying?
There was a problem hiding this comment.
I can bring it back in the next iteration!
| </nav> | ||
| </div> | ||
| <div class="container"> | ||
| <div class="container m-footer-links"> |
There was a problem hiding this comment.
Issue (non-blocking): I'm not sure if it's m-footer-links or some other CSS thing (or just something fiddly in the new DOM), but there's a funky line-wrap in the footer icons at browser widths from 1088 to 1343 px.
(FWIW, the line-wrapping behavior is already wrong and weird in prod - it's just that the language stuff in the footer is making that always happen at certain common breakpoints, whereas in prod, it only happens at very narrow screen widths)
There was a problem hiding this comment.
Thank you for this callout! This may change completely based on designs, but I'll make a note.
|
|
||
| @development_additional_locales [ | ||
| @default_locale, | ||
| @development_locale, |
There was a problem hiding this comment.
Question (non-blocking): Is @development_locale still special, or would it make sense to lump all of the locales together (probably still keeping @default_locale separate)?
There was a problem hiding this comment.
Yeah, good question. I kept @development_locale intact so functionality could remain unchanged when the feature flag was turned on, but we could def merge these down the road.
3 participants
Scope
The current Languages picker is a styled skin for the Google Translate plugin. This PR provides an initial iteration (with styling/UX subject to future change once designs are confirmed) of a new language picker set behind the feature flag for Smartling translations.
We have a .po file for Spanish in the DotCom repo, which I believe is why we're getting the Spanish translations when I select them. The other languages besides English & Spanish don't work yet since we don't have the Smartling translations/files for them in the repo yet.
Coming in future PRs:
Asana Ticket:
https://app.asana.com/1/15492006741476/project/1213994587940653/task/1214847351822159?focus=true
How to test
/_flagsand enable the Smartling translations flag.pofiles for them.Screen.recording.of.languages.menu.6-25.mov