Jb/update storybook theme #246
Conversation
jbouzi12
requested review from
greggb,
jestapinski and
melissarudofsky
as code owners
| @@ -0,0 +1,12 @@ | |||
| <style> | |||
There was a problem hiding this comment.
What area of the storybook do these styles apply to?
.storybook/preview-head.html
Outdated
| <style> | ||
| body { | ||
| font-family: 'Fira Mono', monospace; | ||
| color: '#fff'; |
There was a problem hiding this comment.
I think the storybook html/body styles should match what we have in layout.js
This color isn't working because of the quotes.
.storybook/preview-head.html
Outdated
| } | ||
| .sbdocs-p code, | ||
| .sbdocs-li code { | ||
| color: #fff !important; |
There was a problem hiding this comment.
I don't think we use pure white anywhere on the site. The only exception I see is in link.js where I think I had a placeholder color that never got updated. I see a bunch of #fff in the SB theme, but that doesn't reflect the color we use.
.storybook/theme.js
Outdated
|
|
||
| // Text colors | ||
| textColor: '#fff', | ||
| textInverseColor: colorMap.devWhite, |
There was a problem hiding this comment.
How does this work? These two values are both white, what makes one 'inverse'? I think we should avoid #fff if possible since it's not part of the theme.
.storybook/preview-head.html
Outdated
| @@ -0,0 +1,12 @@ | |||
| <style> | |||
| body { | |||
| font-family: 'Fira Mono', monospace; | |||
There was a problem hiding this comment.
If this is part of the preview area shouldn't it be populated from theme file configs?
.storybook/theme.js
Outdated
| appBorderRadius: 4, | ||
|
|
||
| // Typography | ||
| fontBase: `akzidenz, -apple-system, BlinkMacSystemFont, "Segoe UI", |
There was a problem hiding this comment.
I don't think akzidenz or fira font files are being loaded.
| @@ -42,8 +43,9 @@ import { colorMap, gradientMap, size, fontSize } from '../theme'; | |||
| <Story name="gradients"> | |||
| {() => { | |||
| const Wrapper = styled('div')` | |||
| fontfamily: 'Fira Mono', monospace; | |||
| fontsize: ${fontSize.xsmall}; | |||
| color: #fff; | |||
There was a problem hiding this comment.
Same comments as above, but I'm curious why we need to define these styles in multiple places. Shouldn't the theme be global for storybook?
|
@greggb Due to the issues with The Also, I mainly used the styles in |
|
I found a couple issues around that - do either of these help? |
|
Yea I found these before going back to the cert editor page work as they seemed promising. I have a separate branch where I started implementing some of these suggestions. |
|
Thanks! |
This PR is the first step to customizing and updating the storybook config. I initially planned to include the configuration updates but with shifting priorities I wanted to at least get the theme updates out there so the branch doesn't get stale.
Future updates
mdx