feat(check-language): add info bubble and store language in localStorage #526
Conversation
|
|
||
| const getLanguage = () => { | ||
| if (localStorage.getItem('language')) { | ||
| return localStorage.getItem('language') as Language; |
There was a problem hiding this comment.
We already retrieve the browser language in the language reducer. This would be a good place to also check for the localStorage language. It also has the benefit that it is only evaluated once on app start and not on every render like here.
Also we have to check if the language in the storage is actually one of the available languages similar to the getBrowserLanguage function.
| const setLanguage = (language: Language) => { | ||
| localStorage.setItem('language', language); | ||
|
|
||
| if (localStorage.getItem('language')) { |
There was a problem hiding this comment.
what is this check for? if you set the item first and then check if it's there it should be always true
|
|
||
| const Navigation: FunctionComponent = () => { | ||
| const dispatch = useDispatch(); | ||
| const [showMenu, setShowMenu] = useState(false); | ||
| const [showInfoBubble, setshowInfoBubble] = useState(true); |
There was a problem hiding this comment.
We don't want to show the bubble if there is already a language in local storage right?
Something like that?
const savedLanguage = localStorage.getItem('language');
const [showInfoBubble, setshowInfoBubble] = useState(!Boolean(savedLanguage));| @@ -21,6 +21,14 @@ const LanguageSelector: FunctionComponent<Props> = ({className = ''}) => { | |||
| const selectedLanguage = useSelector(languageSelector); | |||
| const dispatch = useDispatch(); | |||
|
|
|||
| const setLanguage = (language: Language) => { | |||
| localStorage.setItem('language', language); | |||
There was a problem hiding this comment.
It would be cool if the saving to localStorage and the dispatch would be combined so that we don't have to handle the saving manually in multiple places. We could just always save the language to local storage when dispatching the setLanguageAction action in the actions/set-language.ts file
src/scripts/config/main.ts
Outdated
| @@ -78,5 +78,6 @@ export default { | |||
| 'http://twitter.com/intent/tweet?text=ESA%20Climate%20From%20Space&url={currentUrl}' | |||
| }, | |||
| legendImage: `${baseUrlStorage}/legend-images/{variable}.png`, | |||
| downloadUrls | |||
| downloadUrls, | |||
| localStorageLanguage: 'language' | |||
There was a problem hiding this comment.
It's the key and not the language itself. Please name it 'localStorageLanguageKey'
| } | ||
| return language; | ||
| }; | ||
| localStorage.clear(); |
There was a problem hiding this comment.
Why did you add this? Debugging I guess, please remove.
| storedLanguage && | ||
| languages.find( | ||
| language => | ||
| language.substr(0, 2).toLowerCase() === |
There was a problem hiding this comment.
We don't need the substr stuff here. Unlike in the getBrowserLanguage function the language in the localStorage should match the defined languages exactly.
closes #466