AB#174 General Traffic now improvements #5608
Conversation
partisaani
requested review from
Antiik91,
VillePihlava,
hjvuor,
optionsome,
sharhio,
tkalvas and
vesameskanen
| const lang = document.cookie | ||
| .split(';') | ||
| .find(item => item.indexOf('lang=') > -1) | ||
| .split('=')[1]; |
There was a problem hiding this comment.
Is there a better way to get a hold of the current language?
There was a problem hiding this comment.
You should use app/store/PreferencesStore, where the current language is stored:
function component (props, context) {
const lang = context.getStore('PreferencesStore').getLanguage();
}
Actually the current way presented above is some sort of mistake. Stores are used for emitting changes which trigger component rendering. Lang does not change dynamically. UI server sends a cookie at first page load. If user changes the language, new page loading takes place. We could simply store the language to configuration.
| /* MAJOR_CHANGES: { | ||
| fi: 'TODO', | ||
| sv: 'TODO', | ||
| en: 'TODO', | ||
| }, */ |
There was a problem hiding this comment.
It is currently unclear where this "suuret muutokset" link that was requested to be implemented should take the user
| export const useTranslationsContext = () => { | ||
| const intl = useContext(IntlContext); | ||
| if (!intl) { |
There was a problem hiding this comment.
I am not sure why you add new context providers? The old way of accesing config or intl from standard context looks so simple:
function mycomponent(props, context) {
const intl = context.intl;
}
There was a problem hiding this comment.
The old way of accessing context is deprecated in React versions newer than 16, so I think this is a welcoming change
There was a problem hiding this comment.
Futureproofing as @hjvuor said.
Accessing the context via a provider is also simpler in deeply nested components as there's no need to drill down the context prop.
Proposed Changes
Pull Request Check List
Review