Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upComponents: Add Card Component #17963
Conversation
This update enables JS to use the color hex codes defined in Sass. The hex codes are copied and stored as constants (objects). The key/value pairs are accessed through a convenient custom getter function.
This update adds the new Card component with it's various sub-components. A set of stories (Storybook) were added for the Card and it's sub-components for UI development.
This update refactors the import/export method of the Card components to follow current conventions, which allows for better code splitting and tree shaking. Unit tests were added for the Card components, as well as tests for how the sub-components integrate with the primary Card.
And fix border style rendering for Header and Footer
| @@ -63,14 +69,28 @@ export { default as ToolbarButton } from './toolbar-button'; | |||
| export { default as Tooltip } from './tooltip'; | |||
| export { default as TreeSelect } from './tree-select'; | |||
| export { default as IsolatedEventContainer } from './isolated-event-container'; | |||
| export { createSlotFill, Slot, Fill, Provider as SlotFillProvider } from './slot-fill'; | |||
| export { | |||
This comment has been minimized.
This comment has been minimized.
ItsJonQ
Oct 15, 2019
•
Author
Contributor
Hmm! The following changes were from my editor's auto format, which leverages ESLint + Prettier from the project's config.
I can revert this if necessary!
This comment has been minimized.
This comment has been minimized.
gziolo
Oct 17, 2019
Member
Yes, please. All contributors use their own tooling and workflows, if they would use other formatting rules, we would see the same lines converted back and forth. In addition, this only increases the number of lines you have to scan during the review.
|
I was looking at the Storybook and noticed that fonts are not really defined or enforced and I was wondering about the guidelines and how do other design systems address fonts? Do we enforce them in components, or do we rely on a global style? If it's the latter, should we add such global style to Storybook? About the Card component, since it's a base component, I'd love to see it used in Gutenberg as well. Do you think we can replace the |
| @@ -43,6 +44,7 @@ | |||
| "react-dates": "^17.1.1", | |||
| "react-spring": "^8.0.20", | |||
| "rememo": "^3.0.0", | |||
| "styled-components": "^4.4.0", | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ItsJonQ
Oct 16, 2019
Author
Contributor
It's not tiny, but it's not terribly large either (e.g. moment.js)
It's currently at:
45.1kb, minified
16.4kb, minified + gzipped
https://bundlephobia.com/result?p=styled-components@4.4.0
The team is reducing the library size by a lot (and making it faster) in v5:
https://medium.com/styled-components/announcing-styled-components-v5-beast-mode-389747abd987
One of the requirements is that you must be using React 16.8, for hook support. Thankfully, it looks like Gutenberg + @wordpress/components does :)
| export const CardContext = createContext( {} ); | ||
| export const useCardContext = () => useContext( CardContext ); | ||
|
|
||
| export const CardProvider = ( props ) => { |
This comment has been minimized.
This comment has been minimized.
youknowriad
Oct 16, 2019
Contributor
What value does this component add over just using the CardContext.Provider one?
This comment has been minimized.
This comment has been minimized.
ItsJonQ
Oct 16, 2019
Author
Contributor
Good question!
This was a pattern I started using as the Provider became more complex. It was an abstraction to keep the initial entry component (the index.js) code relatively simple. Sorta like how you might have a simple app.js, but a separated complicated Redux action/reducer setup.
Since the current implementation + feature-set is very minimal, I can consolidate the Provider logic into the main index.js file :). No need to pre-optimize - also, the code is clearer
| /** | ||
| * External dependencies | ||
| */ | ||
| import { shallow } from 'enzyme'; |
This comment has been minimized.
This comment has been minimized.
youknowriad
Oct 16, 2019
Contributor
I guess it's not written anywhere, but we've been trying to slowly move away from "enzyme". I think we settled on just using the tools provided by the React team to avoid having tests blocking us from upgrade React in future updates...
cc @gziolo
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ItsJonQ
Oct 16, 2019
Author
Contributor
Oh boy! @gziolo , I didn't know about this thread! But I went over (slash ranted) about testing difficulty with Enzyme on my live coding session
@youknowriad Is there an example of component tests I should follow?
I'm more than happy to refactor/remove the tests if it's inline with the direction and future of @wordpress/component testing
This comment has been minimized.
This comment has been minimized.
gziolo
Oct 17, 2019
•
Member
The usage of update to refresh components is very tricky and it was introduced as a workaround to fix their previous architectural decisions :(
This comment has been minimized.
This comment has been minimized.
gziolo
Oct 17, 2019
Member
Is there an example of component tests I should follow?
I'm more than happy to refactor/remove the tests if it's inline with the direction and future of@wordpress/componenttesting
We probably don't have too many unit tests covering UI components, which use something different than Enzyme. I guess it's fine to keep your tests for the time being and figure out our strategy in the linked issue (#17249).
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
In the parent issue they discuss the block directory feature as a good place to redo it using Card: |
This comment has been minimized.
This comment has been minimized.
|
First, love this! What I love about it is this both solves a component needed and the open way you went about creating it. Also, props on a really thorough PR - I love how detailed it is. From a design perspective this feels as expected, which is great. I can see how it could adapt and be customised to most situations. The minimal nature makes sense because it can iterate. @youknowriad's comment about the panel did get me thinking about where a panel and card differ and even if they should. This feels like a great conversation point. I can see how a panel is a card used in a particular context. |
This comment has been minimized.
This comment has been minimized.
|
I really like this too @ItsJonQ! Great work. Regarding the |
This comment has been minimized.
This comment has been minimized.
|
As a follow up, I'll create a PR to add CodeSandbox demo: |
This comment has been minimized.
This comment has been minimized.
|
bb54a62 - not that much work to switch libraries |
|
LGTM! |
|
I think this PR got enough attention and it was reviewed and tested by several people. Let's call it an experiment and get reevaluate this approach in a few weeks whether CSS in JS works well for the project. The immediate follow-up PR should try to use those newly introduced components in the existing codenase as discussed in the comments in this PR and its parent issue. |
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "presets": [ "@wordpress/babel-preset-default" ], | |||
| "plugins": [ "babel-plugin-emotion" ] | |||
This comment has been minimized.
This comment has been minimized.
gziolo
Nov 13, 2019
Member
This file was moved to storybook/.babelrc - this one should be removed and the other one updated. It's a merge issue.
This comment has been minimized.
This comment has been minimized.
Removed the local babelrc from components. Added babel-emotion to global storybook/.babelrc
| @@ -1,6 +1,7 @@ | |||
| { | |||
| "presets": [ "@wordpress/babel-preset-default" ], | |||
| "plugins": [ | |||
| "babel-plugin-emotion", | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
All good, let's Thank you everyone involved to make this happen! |
This comment has been minimized.
This comment has been minimized.
|
What has happened to Travis, why it didn't start for the last commit? The previous one https://travis-ci.com/WordPress/gutenberg/builds/136300101 looks good. |
This comment has been minimized.
This comment has been minimized.
|
@gziolo Not sure~! Just restarted it |
This comment has been minimized.
This comment has been minimized.
|
Nice work everyone |
| @@ -35,6 +37,7 @@ | |||
| "classnames": "^2.2.5", | |||
| "clipboard": "^2.0.1", | |||
| "dom-scroll-into-view": "^1.2.1", | |||
| "hex-rgb": "^4.1.0", | |||
This comment has been minimized.
This comment has been minimized.
aduth
Nov 21, 2019
Member
hex-rgb uses modern JavaScript (const, let, etc.) and is not transpiled to be able to run in older browsers.
tl;dr: Including this library has broken the editor in Internet Explorer.
Apparently this is an intentional choice on their part: sindresorhus/hex-rgb#10 (comment)
This module mainly targets Node.js, not the browser. It's up to you to transpile it with Babel if you want to use it in the browser.
If it's something we plan to use, it would need to be transpiled as well. I don't recall having had to deal with this for any other dependencies, and it would probably require some work to adapt into our build tooling.
This comment has been minimized.
This comment has been minimized.
jameslnewell
Nov 22, 2019
•
Contributor
Good catch! Long term I think its probably good idea to support this in our tooling just as CRA, Parcel and other tools in the JavaScript ecosystem have done. Switching to polished's parseToRgb or similar fn might be a quick fix.
Edit: I'm working on the quick fix now
Edit: The quick fix is here #18681
This comment has been minimized.
This comment has been minimized.
jameslnewell
Nov 22, 2019
•
Contributor
btw, thinking about the longer-term fix, I saw this on Twitter the other day and it seems that not compiling (or only compiling so-far) is closer to becoming a defacto standard (yay for smaller bundle sizes for apps only supporting modern browsers!).



ItsJonQ commentedOct 15, 2019
•
edited
Description
This update adds a new
<Card />component to@wordpress/components, as well as a collection of flexible sub-components that can be used to composed various user interfaces. The initial sub-components include:<CardBody /><CardDivider /><CardFooter /><CardMedia />https://deploy-preview-17963--gutenberg-playground.netlify.com/design-system/components/?path=/story/card--default
The
<Card />component's designs are very minimal. Theborder-radius,border-color, andpaddingare based on the aesthetics and values found within@wordpress/componentsand Gutenberg.<Card />drew it's component API (and it's composition) from other frameworks/libraries like Bootstrap and Material UI.An example for how a
<Card />UI my be composed looks something like this:How has this been developed & tested?
<Card />was developed entirely in Storybook. The Knobs add-on was added to make developing and testing easier. I've set up knobs for various props and configurations to see how the various sub-components can fit together.Knobs was incredibly valuable in the development of
<Card />. It helped me improve the iteration cycle/testing of Context (which the component uses internally). The Storybook UI allowed me to catch and fix breaking edge-case CSS issues.Lastly, Storybook was very helpful in fine-tuning the
<CardMedia />component, which relies more on responsive behaviour:🧪 Jest / Enzyme
I've written unit tests for all of the various sub-components. I've also added integration tests for sub-components ->😄 (as far as architecture, tech, etc... goes)
<Card />. The naming conventions and testing techniques were drawn from the existing Enzyme based tests I found within@wordpress/components. I didn't add anything newTypes of changes
This is new and experimental! For the
<Card />styles (and it's sub-components), I introduced and used styled-components to take care of the CSS.I've seen a couple of PRs attempting this as well:
#17673
#17614
The benefits of a CSS-in-JS solution are many. Those PRs do a fantastic job at walking through several key benefits - especially for native mobile.
I think it's best feature is... components will be styled and working out-of-the-box. If you build a React component with styled components, you can
importthe component perfectly styled straight fromnode_modules. No fussing with Webpack, Grunt, compilers or scripts. It provides for a wonderful developer experience. Not only that, styled components will provide features like theming. This will make it much easier for folks to consume@wordpress/componentsto build their own interfaces - for their own blocks, plugins, or interfaces outside of Gutenberg.An incredible example of this experience can be found in the Material UI React library. All of the components come pre-styled and can be themed (Note: They use an alternate CSS-in-JS library, not styled components).
This is💯 important. To do this, I added a standard CSS className to the various components (e.g.
components-card-header), just like the rest of@wordpress/components. Although these classNames aren't responsible for direct styling, they enable:htmlcomposition when debugging (e.g. Inspect Element)Since I ventured into CSS-in-JS for the pull request, I needed a way to use the same colour values we have from
.scssbut for.jscode. To do this, I built acolor()utility, which lives within the newcomponents/utilsdirectory.It can be used like this:
The
.notation represents the nested colour values I have stored in the constantcolor.values.jsmap (Object).I (temporarily) updated the😂 ).
block-library/buttoncomponent with the new<Card />component - wrapping it's contents. In addition to working with Storybook and Jest, I needed to make sure the code worked with Gutenberg (of courseIn this test, I was able to see that the Styled Components render components worked with the existing components and styles (and the thing didn't explode).
I coded these components via YouTube livestream this past Friday (and a bit this morning):
Here are the videos, in case anyone is interested:
https://www.youtube.com/watch?v=oAPuOB9oYCw
https://www.youtube.com/watch?v=Kf-CtY3wQGM
My thinking for the live coding was to demonstrate and explain my thinking throughout the design process. Also, it was wonderful to engage with folks on chat. There were several points where we collaborated to figuring out specific component API and behaviour. It was lovely💕 .
I would love to know what you folks think! I understand that some of these approaches are new in the context of
@wordpress/componentsand Gutenberg, specifically Stories and Styled Components. I'm more than happy to answer questions!Thank you so much for your time🙏
Checklist:
Potentially addresses:
#15921