-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/basic i18n #452
Feature/basic i18n #452
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…e new i18n structure, and 2) because they tested the content rather than cricital functionality
…Long term our custom Modal should be replaced with a standard Modal solution.
All previous comments have been resolved. Now ready for further review 🙂 NOTE: The following was tested with DEV and comparing to PROD, which has some obvious differences in performance.I just found another potential issue that is introduced with this PR: listColumns() is called many more times than before. This is because we can't use the constant dataDescriptions, and instead need to get the translated version via getDataDescriptions(), which probably cause extra re-renders. One good thing is that we do have some caching, which greatly speeds things up. But it is still significantly slower than before. To some extent, this is expected since translations require much more work compared to hardcoded strings. But if we think it is too big of a performance difference, we might want to introduce a few useMemo() calls to try to reduce re-renders. Sidenote: when debugging this, it became clear that the dataset definitions have many responsibilities. Either they should have all rendering logic (like which field to display, and how to format it), or we separate this. But let's not do that now. |
…tly. Also improve TypeScript types to prevent this error in the future.
fixes #345
fixes #444
Now ready for review.
Main changes
next-i18next
(based oni18next
) to handle translationsutslappen
as key (to align with the existing URL structure that we need to support), instead ofUtsläppen
. This makes it easier to separate code and logic from content.utils/getServerSideI18n.ts
for more details)Markdown
component based onreact-markdown
, to allow formatting links, bold, italic, and lists via translated content rather than hard coded in JSX.Minor fixes and changes
details
element to makeScorecardSection
accessible and easier to use (for example with keyboard).StyleSheetManager
fromstyled-components
to only render valid HTML attributes to the DOM.<thead>
to fix React error<InfoModal>
by clicking on the backdrop.<InfoModal>
in MunicipalityEmissionGraph.tsxNavigation.tsx
which seems to have been left over in some refactoring.Range.tsx
componentLoadingContainer
fromcomponents/shared.tsx
Notes
The need to provide translations to every Next.js page can be tricky. However, this will not be a problem if/when we migrate to the Next.js app router, since we then will be able to specify the props (like translations) for the root layout, only adding the more specific props (like translations) for child routes. Since the app router will allow us to avoid this issue, I thus opted to prepare for i18n namespaces now while we're at it.
For more details from the process of working on this, please see Add support for basic i18n: refactor strings to separate content from code #345 (comment)