[Migration]: box -> boxNew #3669
Conversation
|
Storybook demo / Chromaticce84e8b13 | 91 components | 135 stories |
|
it's currently missing a change to using |
@navikt/aksel/src/codemod/transforms/darkside/box-to-boxnew/box-to-boxnew.ts
Show resolved
Hide resolved
| if (config?.replacement) { | ||
| attrvalue.value = config.replacement; | ||
| } | ||
| // else { |
There was a problem hiding this comment.
Yea would be nice if we could insert a comment above the line, or just simply at the top of the file with a spesific keyword, and potentially what they need to change 🥇
There was a problem hiding this comment.
having no replacement could mean two things right?
- it's identical, and doesn't need changing?
- it's so different there's no obvious automatic alternative?
There was a problem hiding this comment.
it's identical, and doesn't need changing?
Unsure if there are any of these cases, but might be some of-course. In those cases, we will still need to make sure to migrate to BoxNew since the "new" tokens won't work with Box even when using the same name.
it's so different there's no obvious automatic alternative?
Yes, if there is no "replacement" in the config, there is nothing we can auto-migrate it to 👍 But you might see some of the keys in the config have `comment´ added to them. We can use these comments to guide the user to what they should do to migrate themselves.
Think it should work, token-name keys should match the names used for the props 🤞 But wouldn't hurt to skim over them to make sure. |
| moveAndRenameImport(j, root, { | ||
| fromImport: "@navikt/ds-react", | ||
| toImport: "@navikt/ds-react", | ||
| fromName: "Box", | ||
| toName: "BoxNew", | ||
| }); | ||
|
|
There was a problem hiding this comment.
Migrating to BoxNew should only be necessary if you have used any of propsAffected. If you only have padding, margin etc Box still works with new theme.
There was a problem hiding this comment.
it's not nice to "upgrade" for the future then? 🤔 (i think it wouldn't hurt, but maybe it's more mental overhead than required... at least in the short term?), but the more i think about this, the better it is to switch over early, and all the better if there's no change in behaviour?
There was a problem hiding this comment.
there's always the chance that you start to diverge from easily migrated box into one that has no analogue in the new system.
There was a problem hiding this comment.
When the darkside is set as the default, we will delete BoxNew and just use Box, so BoxNew is mainly just for those testing the new system. Bu i guess you're right, it wont hurt to just migrate to BoxNew always to avoid having both of them used at the same time 👍
Description
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.")