feat: reference component migration #1206

Averethel · 2024-11-15T12:07:40Z

References https://smg-au.atlassian.net/browse/FED-420

Motivation and context

The idea behind this PR is to have a reference when migrating our component library components.
I tried to showcase a few cases of components and refer to the specific examples further in the description.

Caveats

Unfortunately, GitHub doesn't detect file moves, even though I did my best to instruct Git to recognise them (i.e. move the files and then edit them in separate commits). To remedy that, I'll reference specific commits to showcase v2/v3 changes

Showcases

Re-exporting chakra component without theming

Adding more design tokes

adding aspect ratio tokens

Re-exporting chakra component while restricting the available props

Stack migration

Migrating a custom component build from chakra building blocks without custom theming

MissingImage migration

How to review this PR?

I suppose best way is to follow by migrations showcased and listed above. All of the changes are visible in highlighted commits

Averethel · 2024-11-15T15:49:11Z

src/components/box/index.tsx

-import { Box, BoxProps } from '@chakra-ui/react';
-export { BoxProps };
-export default Box;
+export { Box, type BoxProps } from '@chakra-ui/react';


Simple re-export from chakra

Averethel · 2024-11-15T15:49:42Z

src/components/index.ts

@@ -1,2 +1,3 @@
 export * from './icons';
 export * from './themeProvider';
+export * from './box';


Don't forget to export the migrated component

Averethel · 2024-11-15T15:50:26Z

src/components/box/index.stories.tsx

@@ -1,32 +1,31 @@
 import { Meta, StoryObj } from '@storybook/react';

-import { space } from 'src/themes/shared/space';
+import { sharedConfig } from 'src/themes/shared/index';


Tokens in storybook can be accessed directly via the sharedConfig

Averethel · 2024-11-15T15:54:03Z

src/components/box/index.tsx

-import { Box, BoxProps } from '@chakra-ui/react';
-export { BoxProps };
-export default Box;
+export { Box, type BoxProps } from '@chakra-ui/react';


Named export instead of default for better storybook code generation.

Before

<__WEBPACK_DEFAULT_EXPORT__>I am a box</__WEBPACK_DEFAULT_EXPORT__>

After

<Box>I am a box</Box>

This doesn't impact all of the components but we want to apply the same pattern for consistency.
You'd only see the __WEBPACK_DEFAULT_EXPORT__ on a branch deployment, locally it doesn't matter which type of export you use

Averethel · 2024-11-15T16:04:23Z

src/components/aspectRatio/index.stories.tsx

  args: {
    width: 300,
    ratio: 4 / 3,
+    children: 'box',


Storybook complains if we pass a react component as an argument. This is because all arguments need to be serialisable to reconstruct the story state via query params. We can leverage mapping in the argTypes to circumvent that.

More on dealing with complex values

Averethel · 2024-11-15T16:09:24Z

src/themes/shared/tokens/aspectRatios.ts

Either create a new file for a token category that doesn't exist yet or add new tokens to the existing files.

To take a look at all design token categories in default chakra theme you can run:

$ npx chakra eject

This will create theme directory with default chakra theme. You can use it as a reference.

You will need to re-generate the type definitions after adding the tokens. Just run:

$ npm run typegen

Or, if you're changing a lot of tokens and want live updates you can run

$ npm run typegen:watch

Averethel · 2024-11-15T16:11:19Z

src/themes/stories/tokens.stories.tsx

@@ -20,6 +20,26 @@ const conversionToPixels = {
    'Pixel values are only used as a reference for Figma designs. We use rem as units.',
 };

+export const AspectRatios: StoryType = {


If you're adding new token category - please add them to the storybook showcase so we can see the theme easily

lkappeler · 2024-11-22T10:21:59Z

src/themes/shared/tokens/aspectRatios.ts

@@ -0,0 +1,22 @@
+import { defineTokens } from '@chakra-ui/react';
+
+export const aspectRatios = defineTokens.aspectRatios({


Where do they originate?

Those are chakra defaults

Maybe align with UX if we wanna restrict them or they are good as is

lkappeler · 2024-11-22T10:25:04Z

src/components/stack/index.tsx

+  | 'paddingBottom'
+  | 'paddingLeft'
+  | 'paddingRight'
+  | 'separator'


Why was the separator added?

Usage in the projects. We would do <Box as={Flex} separator={...} /> instead of using Flex directly

lkappeler · 2024-11-22T10:25:31Z

src/components/stack/index.tsx

-  | 'marginBottom'
-  | 'marginTop'
-  | 'divider'
+  | 'margin'


Where does the requirement come from to extend all padding and margin props

There's been a change to how margins and paddings behave. In v2 you could pass them as object containing sides:

margin={{ top: 'sm', bottom: 'lg'}

This doesn't work anymore; an unqualified one applies the same margin (or padding) on each side. So to keep the existing functionality I whitelisted all sides explicitly

lkappeler · 2024-11-22T10:35:58Z

src/components/table/index.tsx

+export type TableCellProps = ChakraTableCellProps & { isNumeric?: boolean };
+const Cell: FC<TableCellProps> = ({ isNumeric, children, ...props }) => (
+  <ChakraTable.Cell
+    {...props}
+    {...{ ...(isNumeric ? { 'data-is-numeric': true } : {}) }}
+  >
+    {children}
+  </ChakraTable.Cell>
+);
+Cell.displayName = 'Table.Cell';
+
+export type TableColumnHeaderProps = ChakraTableColumnHeaderProps & {
+  isNumeric?: boolean;
+};
+const ColumnHeader: FC<TableCellProps> = ({
+  isNumeric,
+  children,
+  ...props
+}) => (
+  <ChakraTable.ColumnHeader
+    {...props}
+    {...{ ...(isNumeric ? { 'data-is-numeric': true } : {}) }}
+  >
+    {children}
+  </ChakraTable.ColumnHeader>
+);
+ColumnHeader.displayName = 'Table.ColumnHeader';


I would make it a breaking change and align over backwards compatibility, thinking there will not be 100s of cases where we need that

lkappeler · 2024-11-22T10:44:48Z

src/components/table/table.stories.tsx

+        </Table.Footer>
+      </Table.Root>
+    </Table.ScrollArea>
+  ),

  args: {


Can you add some comments on the changed/extended interfaces and the reasoning behind (don't think it's bad but it can help follow what part are migration changes and which parts are improvements)

Yes, of course. I followed the default table style and applied our customisations (similar to what we did in v2). The main change is that you now have more freedom when defining variants. In v2 you were limited to variant and size properties, in v3 you are free to define all the variations you need under the variant prop and have a fine-grained control over variants interacting with each other via compoundVariants. So I'm following the default configuration and embracing this approach. variant is a global look and feel and then we have different flags (like striped) that can be applied on top of that.

I am wondering if we should restrict the options here. I am not sure UX's intention would be to have so many variations

lkappeler

I went through your example and they are pretty clear. It feels a lot simpler/more straight-forward to work with than the current version.

I would be careful/explicit with improvement changes along with the migration, it makes it a lot harder to follow the context of wether it's required for some changes from v2 to v3 or something we can improve in our implementation

lkappeler

Regarding the chat we had this morning I do think we can merge this back to the stack root. Maybe check with UX on the restrictions but we can also do that down the road.

automotiveengineeringbot · 2024-11-26T14:44:24Z

🎉 This PR is included in version 23.12.0-chakra-v3-b007baa34b3042d13cb9ebf96bd863b4c5b0e151.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

automotiveengineeringbot · 2024-11-26T16:39:05Z

🎉 This PR is included in version 23.15.0-chakra-v3-f0a29c4689b248124d11f182c1701bad4af03971.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

automotiveengineeringbot · 2024-12-05T13:34:12Z

🎉 This PR is included in version 23.16.0-chakra-v3-1179149485ac09b9c262ae8648e855b1ce506b02.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

automotiveengineeringbot · 2024-12-10T07:33:50Z

🎉 This PR is included in version 23.16.0-chakra-v3-7a58fab7dfb141d07161cc692ea467088dbf00b4.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

automotiveengineeringbot · 2024-12-11T12:44:28Z

🎉 This PR is included in version 23.17.0-chakra-v3-ed38805868813712826f68f9fd6cbcde5f1c3db6.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

automotiveengineeringbot deployed to branch-components-pkg-chakra-v3-reference-migration November 15, 2024 12:12 Active

Averethel force-pushed the chakra-v3/reference-migration branch from a227d43 to 653356b Compare November 15, 2024 15:14

automotiveengineeringbot deployed to branch-components-pkg-chakra-v3-reference-migration November 15, 2024 15:20 Active

Averethel added 10 commits November 15, 2024 16:23

chore: move v2 box to v3

7b639eb

feat: migrate Box

3457a79

chore: copy over the aspect ratio

8895690

feat: migrate AspectRatio

4e3e81d

feat: add aspect ratio tokens

3c9ff0b

chore: move stack

1bad097

feat: migrate stack

fef6f10

chore: controls sort for Box

44eb721

fix: display name for AspectRatio

170e30e

chore: move grid

d5f50e5

Averethel force-pushed the chakra-v3/reference-migration branch from 653356b to f713e8e Compare November 15, 2024 15:39

Averethel added 7 commits November 15, 2024 16:44

feat: migrate grid

86a191e

chore: move missing image

974f020

feat: migrate missing image

6b9c27e

chore: move vehicle reference

84dfc2d

feat: migrate vehicle reference

e522140

fix: rollup doesn't like circular dependencies

b49c37b

chore(typing): design types in storybook

e099ce3

Averethel force-pushed the chakra-v3/reference-migration branch from f713e8e to e099ce3 Compare November 15, 2024 15:45

Averethel commented Nov 15, 2024

View reviewed changes

automotiveengineeringbot deployed to branch-components-pkg-chakra-v3-reference-migration November 15, 2024 15:51 Active

Averethel commented Nov 15, 2024

View reviewed changes

Averethel requested review from lkappeler, filip-kis-smg and marko-markovic-smg November 19, 2024 16:21

Averethel marked this pull request as ready for review November 19, 2024 16:21

Averethel mentioned this pull request Nov 19, 2024

feat: Migrate to chakra-ui v3 #1185

Draft

100 tasks

Merge branch 'chakra-v3/root' into chakra-v3/reference-migration

4c8add3

automotiveengineeringbot deployed to branch-components-pkg-chakra-v3-reference-migration November 20, 2024 08:17 Active

chore: adapt storybook to shared config

2f18da3

automotiveengineeringbot deployed to branch-components-pkg-chakra-v3-reference-migration November 20, 2024 10:06 Active

lkappeler reviewed Nov 22, 2024

View reviewed changes

lkappeler approved these changes Nov 26, 2024

View reviewed changes

Averethel added 3 commits November 26, 2024 14:33

Merge branch 'chakra-v3/root' into chakra-v3/reference-migration

ca88747

chore: adjust the migrated stories to improved storybook setup

a67168d

fix: turns out module setup works better than tsconfig-paths

f00f22f

automotiveengineeringbot deployed to branch-components-pkg-chakra-v3-reference-migration November 26, 2024 13:49 Active

Averethel added 2 commits November 26, 2024 14:51

chore: remove isNumeric prop

4086772

chore: starting point for documenting migration changes

4d88158

Averethel force-pushed the chakra-v3/reference-migration branch from 1c285bb to 4d88158 Compare November 26, 2024 14:28

automotiveengineeringbot deployed to branch-components-pkg-chakra-v3-reference-migration November 26, 2024 14:33 Active

Averethel merged commit b007baa into chakra-v3/root Nov 26, 2024
10 checks passed

Averethel deleted the chakra-v3/reference-migration branch November 26, 2024 14:39

automotiveengineeringbot added the released on @chakra-v3/root label Nov 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: reference component migration #1206

feat: reference component migration #1206

Averethel commented Nov 15, 2024 •

edited

Loading

Averethel Nov 15, 2024

Averethel Nov 15, 2024

Averethel Nov 15, 2024

Averethel Nov 15, 2024 •

edited

Loading

Averethel Nov 15, 2024

Averethel Nov 15, 2024

Averethel Nov 15, 2024

lkappeler Nov 22, 2024

Averethel Nov 22, 2024

lkappeler Nov 26, 2024

lkappeler Nov 22, 2024

Averethel Nov 22, 2024

lkappeler Nov 22, 2024

Averethel Nov 22, 2024

lkappeler Nov 22, 2024

lkappeler Nov 22, 2024

Averethel Nov 22, 2024

lkappeler Nov 26, 2024

lkappeler left a comment

lkappeler left a comment

automotiveengineeringbot commented Nov 26, 2024

automotiveengineeringbot commented Nov 26, 2024

automotiveengineeringbot commented Dec 5, 2024

automotiveengineeringbot commented Dec 10, 2024

automotiveengineeringbot commented Dec 11, 2024

		@@ -0,0 +1,22 @@
		import { defineTokens } from '@chakra-ui/react';

		export const aspectRatios = defineTokens.aspectRatios({

feat: reference component migration #1206

feat: reference component migration #1206

Conversation

Averethel commented Nov 15, 2024 • edited Loading

Motivation and context

Caveats

Showcases

Re-exporting chakra component without theming

Adding more design tokes

Re-exporting chakra component while restricting the available props

Migrating a custom component build from chakra building blocks without custom theming

Migrating a component with defined styling

Migrating a custom component with multipart styling

Migrating a chakra component that is now a composite component

How to review this PR?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Averethel Nov 15, 2024 • edited Loading

Choose a reason for hiding this comment

Before

After

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lkappeler left a comment

Choose a reason for hiding this comment

lkappeler left a comment

Choose a reason for hiding this comment

automotiveengineeringbot commented Nov 26, 2024

automotiveengineeringbot commented Nov 26, 2024

automotiveengineeringbot commented Dec 5, 2024

automotiveengineeringbot commented Dec 10, 2024

automotiveengineeringbot commented Dec 11, 2024

Averethel commented Nov 15, 2024 •

edited

Loading

Averethel Nov 15, 2024 •

edited

Loading