chore: change the color palette #48

marcosh-ramos · 2025-01-17T17:48:58Z

closes #35
closes #45

What?

With this task we are changing the colors to use the new variants introduced by storyblok.

Why?

How to test? (optional)

Open the preview and check the palette of colors and how it looks in the components.

vercel · 2025-01-17T17:49:03Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
mui	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 24, 2025 2:28pm

marcosh-ramos · 2025-01-17T17:53:34Z

src/storybook/design/Shadows.mdx

@@ -28,7 +28,7 @@ export const Shadows = () => {
                    Display
                </Typography>
            </Grid>
-            {Object.keys(theme.shadows).map((index) => (
+            {Object.keys(theme.shadows.slice(0, 5)).map((index) => (


did this because for the design system we only have 4 variants of shadow

@marcosh-ramos, this will cause a breaking change though. The MUI theme type accepts 25 variants, so we must adhere to it.

At a later point, we could create a custom theme, but for now we must adhere to the type.

Perhaps we can make every 5 shadows the same?

0-4 -> 0

5-9 -> 1

etc

I think that would be strange have the shadows from 0 to 4 mapping to none, at least was not was I would expect if I use shadows[1]

I went back to what was before. for me would be worst to map like this because people that now use the shadows from 1 to 4 would lose it with this update.

changes:
e66a71c#diff-eaaeb6908dd041479365a278ba68513d6ffb3512501d44999c6e1685cfa89cb1L31
e66a71c#diff-f2727474dc5684e74bd35e134abe55e2a53454471a4f806c72942be9b1b0b458R56-R67

SiloGecho97

Looks good to me

johannes-lindgren

@marcosh-ramos Nice! I found some breaking changes though.

We'd need a more detailed better description, so that it's clear what we are updating. We will need to communicate all of this in detail in the release notes anyways.

johannes-lindgren · 2025-01-20T11:54:42Z

src/storybook/design/Shadows.mdx

@@ -28,7 +28,7 @@ export const Shadows = () => {
                    Display
                </Typography>
            </Grid>
-            {Object.keys(theme.shadows).map((index) => (
+            {Object.keys(theme.shadows.slice(0, 5)).map((index) => (


@marcosh-ramos, this will cause a breaking change though. The MUI theme type accepts 25 variants, so we must adhere to it.

At a later point, we could create a custom theme, but for now we must adhere to the type.

Perhaps we can make every 5 shadows the same?

0-4 -> 0

5-9 -> 1

etc

johannes-lindgren · 2025-01-20T11:56:38Z

src/theme/design-tokens/index.ts

 //Box-shadow
 export const box_shadow_default = `0 2px 17px 3px rgba(34, 42, 69, 0.07)`
 //Transition
 export const base_transition_duration = '0.1s'
 export const base_transition_easing = 'ease-in-out'
 export const base_transition = `all ${base_transition_duration} ${base_transition_easing}`
-export const base_border_radius = 5
+export const base_border_radius = 8


This seems a bit high. It would mean mean that the radius cannot go below 8. Is this what we want?

this is what we use as the default in storyfront

@marcosh-ramos maybe so, but it means we cannot really go below it:

"...but it means we cannot really go below it", I din't understand why, so in this case with what we have now we can't go below 5? Anyway, I went back to what was before (5), since this PR was to change the colors.

change: e66a71c#diff-cc30f115439d4b426941a0f00ad2d2cd110eb3602cf16f906a68460407a389c4L19

johannes-lindgren · 2025-01-20T11:57:02Z

src/theme/design-tokens/palette.ts

This would be a breaking change. We cannot change the variable names

Why a breaking change? is somebody importing directly from this file?

@marcosh-ramos if someone imports it from

import { color_primary } from '@storyblok/mui'

This change would break their app

I went back with the old colors variables e66a71c#diff-e6dca4a2cd646e67ccaaa1d6e49d8d017a71dbba22ba47fa69ad4ddb96119156

johannes-lindgren · 2025-01-20T11:59:54Z

src/theme/design-tokens/palette.ts

+export const sb_primary_50 = '#d5fcfa'
+export const sb_primary_100 = '#b5ece9'
+export const sb_primary_200 = '#9ae6e2'
+export const sb_primary_300 = '#72dfda'
+export const sb_primary_400 = '#37dcd3'
+export const sb_primary_500 = '#04c8c0'
+export const sb_primary_600 = '#00b3b0'
+export const sb_primary_700 = '#05807f'
+export const sb_primary_800 = '#0a6465'
+export const sb_primary_900 = '#0a6465'
+export const sb_primary_950 = '#003033'
+export const sb_primary = sb_primary_700


If we keep this, we could put it in an object dictionary, like the grey.

const primary = { 50: ..., 100: ..., }

etc

Also, I am not sure we need the full gradient. It kind of defeats the purpose of designs tokens. Doesn't it?

I just map the colors in the same way we have in design system https://storyfront-design-system-git-revert-7000-f-5b1d72-storyblok-com.vercel.app/?path=/docs/foundations-colors--documentation

changes made e66a71c#diff-e6dca4a2cd646e67ccaaa1d6e49d8d017a71dbba22ba47fa69ad4ddb96119156

johannes-lindgren · 2025-01-20T12:00:45Z

src/theme/light-theme.tsx

+  '0px 4px 8px 0px rgba(27, 36, 63, 0.08)',
+  '0px 8px 24px 0px rgba(27, 36, 63, 0.08)',
+  '0px 16px 32px 0px rgba(27, 36, 63, 0.08)',
+  '0px 16px 48px 0px rgba(27, 36, 63, 0.08)',
+  ...Array(20).fill('none'),


See my suggestion above to make every 5 shadows the same, rather than making shadows[20-24] none

see #48 (comment)

johannes-lindgren · 2025-01-20T12:02:23Z

src/theme/light-theme.tsx

-    main: color_primary,
-    contrastText: white,
+    main: sb_primary,
+    contrastText: sb_neutral_white,


What is the design token's definition of "neutral white" is it for text or surfaces? "neutral black" doesn't seem to be intended for text, for example.

there there's no definition, it's only a primitive colors for black and white

Then we could just hard-code it: #ffffff. If the design token doesn't have a semantic meaning, what is the point?

I went back with the way it was e66a71c#diff-f2727474dc5684e74bd35e134abe55e2a53454471a4f806c72942be9b1b0b458R76

src/theme/design-tokens/palette.ts

johannes-lindgren · 2025-01-20T12:03:44Z

src/theme/light-theme.tsx

  grey: {
-    '50': '#f7f8f9',
-    '100': '#ECF0FF',
-    '200': '#E2E6F5',
-    '300': '#D1D5E4',
-    '400': '#ADB1C0',
-    '500': '#8d919f',
-    '600': '#656976',
-    '700': '#525662',
-    '800': '#343743',
-    '900': '#141822',
+    '50': sb_base_50,
+    '100': sb_base_100,
+    '200': sb_base_200,
+    '300': sb_base_300,
+    '400': sb_base_400,
+    '500': sb_base_500,
+    '600': sb_base_600,
+    '700': sb_base_700,
+    '800': sb_base_800,
+    '900': sb_base_900,
  },


We could perhaps move this whole object to design-tokens/palette

it's possible, just a point, in design tokens there's on more color (950) that we could not pass here since the mui only maps until 900

@marcosh-ramos, the object could still contain it. For grey, we could create a new object where we just remove the 950

done e66a71c#diff-e6dca4a2cd646e67ccaaa1d6e49d8d017a71dbba22ba47fa69ad4ddb96119156R132

src/theme/design-tokens/palette.ts

johannes-lindgren · 2025-01-20T12:09:50Z

src/theme/light-theme.tsx

            },
          },
+          '&:hover': {
+            backgroundColor: sb_secondary_50,


For these properties, we should use ({ theme }) => to use the design token from the palette. At the time that I wrote this existing line, I did not know this was possible.

done e66a71c#diff-f2727474dc5684e74bd35e134abe55e2a53454471a4f806c72942be9b1b0b458R656

- coming back with the old color variables - coming back with the 5px to the base_border_radius - coming back with the old shadows

marcosh-ramos added 2 commits January 17, 2025 14:43

chore: updating the colors to the new variants of storyblok

72547b8

chore: change shadows to folow the storyblok design system

74cf601

marcosh-ramos changed the title ~~Change the color palette~~ chore: change the color palette Jan 17, 2025

marcosh-ramos requested review from johannes-lindgren and SiloGecho97 January 17, 2025 17:52

marcosh-ramos commented Jan 17, 2025

View reviewed changes

vercel bot deployed to Preview January 17, 2025 17:56 View deployment

chore: change button height and padding

2837e37

vercel bot deployed to Preview January 17, 2025 18:10 View deployment

SiloGecho97 approved these changes Jan 18, 2025

View reviewed changes

johannes-lindgren requested changes Jan 20, 2025

View reviewed changes

chore: pull request changes

e66a71c

- coming back with the old color variables - coming back with the 5px to the base_border_radius - coming back with the old shadows

vercel bot deployed to Preview January 24, 2025 14:28 View deployment

marcosh-ramos requested a review from johannes-lindgren January 24, 2025 15:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: change the color palette #48

chore: change the color palette #48

marcosh-ramos commented Jan 17, 2025 •

edited

Loading

vercel bot commented Jan 17, 2025 •

edited

Loading

marcosh-ramos Jan 17, 2025

johannes-lindgren Jan 20, 2025

marcosh-ramos Jan 20, 2025

marcosh-ramos Jan 24, 2025 •

edited

Loading

SiloGecho97 left a comment

johannes-lindgren left a comment

johannes-lindgren Jan 20, 2025

johannes-lindgren Jan 20, 2025

marcosh-ramos Jan 20, 2025

johannes-lindgren Jan 21, 2025

marcosh-ramos Jan 24, 2025 •

edited

Loading

johannes-lindgren Jan 20, 2025

marcosh-ramos Jan 20, 2025

johannes-lindgren Jan 21, 2025

marcosh-ramos Jan 24, 2025

johannes-lindgren Jan 20, 2025

johannes-lindgren Jan 20, 2025

marcosh-ramos Jan 20, 2025

marcosh-ramos Jan 24, 2025

johannes-lindgren Jan 20, 2025

marcosh-ramos Jan 24, 2025

johannes-lindgren Jan 20, 2025

marcosh-ramos Jan 21, 2025

johannes-lindgren Jan 21, 2025

marcosh-ramos Jan 24, 2025

johannes-lindgren Jan 20, 2025

marcosh-ramos Jan 21, 2025

johannes-lindgren Jan 21, 2025

marcosh-ramos Jan 24, 2025

johannes-lindgren Jan 20, 2025

marcosh-ramos Jan 24, 2025

chore: change the color palette #48

Are you sure you want to change the base?

chore: change the color palette #48

Conversation

marcosh-ramos commented Jan 17, 2025 • edited Loading

What?

Why?

How to test? (optional)

vercel bot commented Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcosh-ramos Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

SiloGecho97 left a comment

Choose a reason for hiding this comment

johannes-lindgren left a 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

marcosh-ramos Jan 24, 2025 • edited Loading

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

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

marcosh-ramos commented Jan 17, 2025 •

edited

Loading

vercel bot commented Jan 17, 2025 •

edited

Loading

marcosh-ramos Jan 24, 2025 •

edited

Loading

marcosh-ramos Jan 24, 2025 •

edited

Loading