[docs] Add "Building design system components" guide with Pigment CSS #41635
Conversation
siriwatknp
added
docs
| if (_source.startsWith('@pigment-css/react')) { | ||
| return require.resolve(`../exports/${tag}`); | ||
| } | ||
| return null; |
There was a problem hiding this comment.
@brijeshb42 without this change, all imports are considered pigment related APIs. import * as React from 'react'; is not possible in the input file.
There was a problem hiding this comment.
All ears to any better solution.
Netlify deploy previewhttps://deploy-preview-41635--material-ui.netlify.app/ Bundle size report |
packages/pigment-css-react/tests/theme/fixtures/themed-component.output.css
Outdated
Show resolved
Hide resolved
packages/pigment-css-react/README.md
Outdated
|
|
||
| ### 3. Style the slot with ownerState | ||
|
|
||
| When you need to style the slot-based props or internal state, wrap them in the `ownerState` object and pass it to each slot as a prop. |
There was a problem hiding this comment.
onwerState is MUI specific primitive. We should also mention that users get access to the direct props as well.
There was a problem hiding this comment.
I think ownerState is also Pigment specific. It's in all of the Pigment codebase. By using ownerState, you don't have to write shouldForwardProp all the time. It makes a lot of sense to me to mention ownerState for styling.
We should also mention that users get access to the direct props as well.
Yes, we can do that.
There was a problem hiding this comment.
It's added only to support material-ui. Otherwise, it's not needed and has nothing to do with Pigment itself.
There was a problem hiding this comment.
Okay, I won't mention ownerState for now but I feel that developers will have to come up with some name anyway, so why not use what we already have?
It's not just for Material UI but quite common use cases. I don't think anyone would like to deal with shouldForwardProp, it's pretty annoying. For me, I just need Pigment to tell me what prop name I should put info for styling into and that's done. I don't want to be aware of props spreading to DOM.
packages/pigment-css-react/README.md
Outdated
| name: 'MuiStat', | ||
| slot: 'root', | ||
| - })(({ theme }) => ({ | ||
| + })(({ theme, ownerState }) => ({ |
There was a problem hiding this comment.
The addition/deletion seems reverse
packages/pigment-css-react/README.md
Outdated
| // framework config file, for example next.config.js | ||
| const { withPigment } = require('@pigment-css/nextjs-plugin'); | ||
|
|
||
| const theme = { |
There was a problem hiding this comment.
Maybe we can also emphasize that library authors can also export their pre configured theme that can be directly imported and passed to the config.
There was a problem hiding this comment.
I decided not to mention the theme configuration at the beginning. I think the theme should be added after teaching how to create a component.
Co-authored-by: Sam Sycamore <[email protected]> Signed-off-by: Siriwat K <[email protected]>
github-actions
bot
added
the
PR: out-of-date
github-actions
bot
removed
the
PR: out-of-date
siriwatknp
changed the title
packages/pigment-css-react/tests/theme/fixtures/themed-component.input.js
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,50 @@ | |||
| import { styled as _styled3 } from '@pigment-css/react'; | |||
There was a problem hiding this comment.
This file might not have been formatted correctly and will later report issue in CI. Can you verify? After prettier, imports to the same module are consolidated into one.
There was a problem hiding this comment.
I run test:update but it remains the same.
There was a problem hiding this comment.
If you look at styled.output.js, it becomes
import { styled as _styled, styled as _styled2, styled as _styled3 } from '@pigment-css/react';There was a problem hiding this comment.
So I am not sure what is correct.
packages/pigment-css-react/tests/theme/fixtures/themed-component.input.js
Outdated
Show resolved
Hide resolved
I use this opportunity to add a test the API altogether.
closes #41537