Skip to content
This repository was archived by the owner on Jan 16, 2022. It is now read-only.

feat(lng): Added change language on the fly#456

Merged
juanpicado merged 11 commits intomasterfrom
feat/change-language-on-the-fly
Apr 23, 2020
Merged

feat(lng): Added change language on the fly#456
juanpicado merged 11 commits intomasterfrom
feat/change-language-on-the-fly

Conversation

@priscilawebdev
Copy link
Copy Markdown
Contributor

@priscilawebdev priscilawebdev commented Apr 5, 2020

Change Language on the fly

image

image

@priscilawebdev priscilawebdev changed the title WIP feat(lng): Added change language on the fly feat(lng): Added change language on the fly Apr 8, 2020
@juanpicado juanpicado added the wip label Apr 8, 2020
@priscilawebdev priscilawebdev force-pushed the feat/change-language-on-the-fly branch from afc0175 to 1318774 Compare April 12, 2020 16:20
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 13, 2020

Codecov Report

Merging #456 into master will decrease coverage by 0.80%.
The diff coverage is 69.64%.

@@            Coverage Diff             @@
##           master     #456      +/-   ##
==========================================
- Coverage   84.22%   83.41%   -0.81%     
==========================================
  Files         141      142       +1     
  Lines        1014     1067      +53     
  Branches      210      211       +1     
==========================================
+ Hits          854      890      +36     
- Misses        127      145      +18     
+ Partials       33       32       -1     
Impacted Files Coverage Δ
src/App/load-dayjs-locale.ts 25.92% <ø> (ø)
src/components/Header/HeaderRight.tsx 79.16% <ø> (ø)
src/components/Icon/Icon.tsx 100.00% <ø> (ø)
src/design-tokens/ThemeContext.ts 100.00% <ø> (ø)
src/components/LanguageSwitch/LanguageSwitch.tsx 63.04% <63.04%> (ø)
i18n/config.ts 100.00% <100.00%> (ø)
src/design-tokens/ThemeProvider.tsx 100.00% <100.00%> (ø)
src/muiComponents/MenuItem/MenuItem.tsx 100.00% <100.00%> (ø)
src/utils/package.ts 96.00% <100.00%> (ø)
src/components/Icon/styles.ts 88.88% <0.00%> (ø)
... and 1 more

Copy link
Copy Markdown
Member

@juanpicado juanpicado left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great ! But few things I think worth to highlight

  • We don't persist the language, on refresh I lose my previous selection, as we do with dark mode.
  • Mobile view does not look fine, the menu + dark mode does not fit.

What do you think?

@juanpicado
Copy link
Copy Markdown
Member

I just merged French, I think we need also to update it here.

@priscilawebdev priscilawebdev force-pushed the feat/change-language-on-the-fly branch from 119d918 to d52a2a5 Compare April 19, 2020 18:36
@priscilawebdev
Copy link
Copy Markdown
Contributor Author

priscilawebdev commented Apr 19, 2020

@juanpicado thanks for the nice feedback! I just added the current language to localStorage, so it should be fine now if you reload the page ...

I also updated the menu background color in dark mode 😉

Regarding the header on mobile devices, I plan to refactor it by adding a mobile menu, because atm it is impossible to add another icon / information ... as you said, it looks odd ... I will do this in a follow-up PR ... but in the meantime, the component switchLanguage will be hidden on smaller devices

I'm not having much time lately, but I will try to do that as soon as I have some free time 🙂

@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@verdacciobot
Copy link
Copy Markdown

Thanks for your PR, the @verdaccio/ui package will be accessible from here for testing purposes:

npm install @verdaccio/ui-theme@v1.5.1-67bd520-pr456.0 --registry https://registry.verdaccio.org

@juanpicado
Copy link
Copy Markdown
Member

@juanpicado thanks for the nice feedback! I just added the current language to localStorage, so it should be fine now if you reload the page ...

I also updated the menu background color in dark mode 😉

Regarding the header on mobile devices, I plan to refactor it by adding a mobile menu, because atm it is impossible to add another icon / information ... as you said, it looks odd ... I will do this in a follow-up PR ... but in the meantime, the component switchLanguage will be hidden on smaller devices

I'm not having much time lately, but I will try to do that as soon as I have some free time 🙂

Perfect, good solution 👍

Copy link
Copy Markdown
Member

@juanpicado juanpicado left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome 💯

@juanpicado juanpicado merged commit 675ee98 into master Apr 23, 2020
@delete-merged-branch delete-merged-branch bot deleted the feat/change-language-on-the-fly branch April 23, 2020 06:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants