DatePicker/MonthPicker: add support for translations provider #3390
Conversation
🦋 Changeset detectedLatest commit: 13d0ca8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Storybook demo / Chromatic📝 Endringer til review: 5821b8b3ecd | 91 komponenter | 135 stories |
| @@ -102,4 +102,24 @@ export default { | |||
| reset: "Tilbakestill tidsperspektiv", | |||
| }, | |||
| }, | |||
| DatePicker: { | |||
There was a problem hiding this comment.
The translation object name is not ideal since it contains texts for both DatePicker and MonthPicker. Some are shared as well. Should we call it something else?
There was a problem hiding this comment.
Can't really think of something different that makes sense, so DatePicker is probably fine. You are technically picking a date when you pick a month or year, so it still technically works in that sense 🙈
| @@ -97,4 +97,24 @@ export default { | |||
| reset: "Tilbakestill tidsperspektiv", | |||
| }, | |||
| }, | |||
| DatePicker: { | |||
| close: "Lukk", | |||
There was a problem hiding this comment.
Is this generic enough to be under "global" ?
There was a problem hiding this comment.
Nvm see the todo under nb.json now. I think it makes sense to just add it to global 👍
There was a problem hiding this comment.
Yes, probably. I will review all the TODOs after all translations has been added.
| nextMonth: "Gå til neste måned", | ||
| previousMonth: "Gå til forrige måned", |
There was a problem hiding this comment.
Does it make sense to prefix them like this to better indicate its intention?
| nextMonth: "Gå til neste måned", | |
| previousMonth: "Gå til forrige måned", | |
| goToNextMonth: "Gå til neste måned", | |
| goToPreviousMonth: "Gå til forrige måned", |
| month: "Måned", | ||
| nextMonth: "Gå til neste måned", | ||
| previousMonth: "Gå til forrige måned", | ||
| year: "År", |
There was a problem hiding this comment.
More of a general question, but the keys day, month and year are quite generic. Would it make sense to have a "global" date key on the same level as "global"?
"date": {
year...
...
}
There was a problem hiding this comment.
Not sure. I want to be restrictive about making keys global since it complicates tings a little. (Have to useI18n() multiple times, and makes the translations prop on the component more complicated.)
Can you think of any benefits to this, other than that it avoids breaking changes if we need to make them global later?
(Side note: I wonder how hard it would be to support multiple "component names" in useI18n() 🤔)
There was a problem hiding this comment.
See no real benefits other than that if felt a little off that you define how "year" is written inside datepicker, so works fine as is for me 👍
| nextYear: "Go to next year", | ||
| previousYear: "Go to previous year", |
There was a problem hiding this comment.
Same as goToNextMonth comment further down, does it make sense to prefix these with goTo to show intention better?
Maybe it should have been a separate PR, but I removed
pointerEvents="none"on CalendarIcon so that you get a tooltip on hover. It also makes NVDA announce the title on hover, which I guess is a plus. (It normally announces everything you hover.)Please help me check that I haven't missed any texts 😅
Component Checklist 📝
@navikt/core/css/config/_mappings.js)@navikt/core/css/tokens.json)@navikt/aksel-stylelint/src/deprecations.ts)@navikt/core/react/src/index.tsand@navikt/core/react/package.json)@navikt/core/css/index.css)<Component>: <gitmoji?> <Text>.E.g. "Button: ✨ Add feature xyz.")