emotion convertion #305
emotion convertion #305
Conversation
# Conflicts: # yarn.lock
# Conflicts: # src/components/Badge.tsx # src/components/ButtonToggle.tsx # src/components/Select.tsx # src/components/tooltip/ListItem.tsx
|
Overall, looks good! I'm confident we can proceed. |
src/components/AvatarList.tsx
Outdated
| @@ -25,6 +25,19 @@ const UserEllipses = styled.li` | |||
|
|
|||
| const User = styled.li` | |||
| display: inline-flex; | |||
|
|
|||
| &:not(:first-child) { | |||
There was a problem hiding this comment.
Will these components never be SSR'd? I thought emotion didn't like :first-child, :nth-child, etc. At the very least, I'm pretty sure this will log warnings in the console.
There was a problem hiding this comment.
yes, this might produce warnings in the console - since you don't expect this design system to be consumed publicly I think it's acceptable to just wrap your application in the CacheProvider with a "compat" cache. This allows you to turn off those warnings for the whole app - if you are going to implement SSR you probably won't use streaming anyway so you can use @emotion/server to extract things and construct the final HTML in a way that those warnings won't be relevant for you (you won't have problems by using that kind of selectors).
Keep in mind that React is working on the new API now (for React 18) that might allow us to circumvent this issue altogether. My hope is that it will allow us to stream without creating the :first-child~ problem. You can check out this PR to get some more information: facebook/react#21913
There was a problem hiding this comment.
Sounds good. Thanks for the info, Mateusz.
Fwiw, while it may not be promoted as such, the SB design system is consumed publicly. I know because I was doing so, in the project I was working on before I joined the team here. It's a great way to make things feel Storybook-native.
There was a problem hiding this comment.
If this is also used publicly then I would recommend making this a major change either way because SSRing strategies might change, some subtle differences in browsers might also be present.
As to the whole warning thing - it depends on how do you want to handle that and what kind of guarantees/recommendations do you want to give to your consumers. For instance "setting up" doc that would describe the CacheProvider setup and the SSR strategy could be OK if you think that this is enough. If you want to avoid that (but you can't easily avoid the latter - SSR strategies) then you can use the silencing comment, like you are already doing in the main Storybook repository.
I understand that this issue is somewhat annoying for people relying on those kinds of selectors - but hopefully it won't be a problem in the future (as mentioned in the previous comment).
# Conflicts: # src/components/tag/TagItem.tsx # src/components/tag/TagList.tsx
- pulled in breadcrumb from frontpage repo
- add story for custom colored icon
- don't forward block, color props
…-into-ds-update-styles 333 - Pull Breadcrumb from `frontpage` into the DS
- added heading story
…ing-from-frontpage-into Pull Heading from `frontpage` into the DS
- add marketing subheading - add marketing subheading story
- add muted prop to existing subheading - add muted subheading story
…eading-from-frontpage Pull AddonsSubheading from `frontpage` into the DS
…from-subheading Remove letter spacing from Subheading muted styles
- update imports - begin copying over footer from frontpage
- began abstracting repeated code - accepting props to generate footer data
- create resource component to remove dup code - add mailinglist todo
- remove props that aren't needed for stuff that's consistent
…e-footer-from-frontpage Pull (most of) Footer from `frontpage` into the DS
- remove unneeded stories
…op-from-ds-icon Remove props from Icon
|
@kylegach @winkerVSbecks What's the process of releasing this as a new major version? Is it easy for either of you to do? Will merging this cause it the be released as a new major automatically? |
I believe it's based on labels. I just added a "Major" label, so it should do the right thing when merged. Someone should confirm my understanding on that, though. |
|
🚀 PR was released in |
@Andarist I made this PR to check if chromatic UI review will report 0 changes as well
📦 Published PR as canary version:
6.4.2-canary.305.ab1cc56.0✨ Test out this PR locally via:
npm install @storybook/design-system@6.4.2-canary.305.ab1cc56.0 # or yarn add @storybook/design-system@6.4.2-canary.305.ab1cc56.0