docs(charts): Adds developer notes to about-charts page#4488
docs(charts): Adds developer notes to about-charts page#4488nicolethoen merged 2 commits intopatternfly:mainfrom
Conversation
|
@mcoker can you review when you have time? I'm curious if it's clear enough, especially since I mention both the React tokens and the standard design tokens. Do you think we need to expand more on if/when to use the chart design tokens instead of or in addition to the React tokens? And if so, that's an area I think I could use clarification too 😊 |
|
|
||
| ## Develop with charts | ||
|
|
||
| Default styles in the [@patternfly/react-charts package](https://www.npmjs.com/package/@patternfly/react-charts) are aligned with our light theme. To use chart styles, you must also have the [@patternfly/patternfly package](https://www.npmjs.com/package/@patternfly/patternfly) installed. |
There was a problem hiding this comment.
I'm a little confused about what it means to "use chart styles". Can you elaborate on that a bit? Just so it's clear (it probably is, just want to make sure) - @patternfly/react-charts in light theme works just fine without @patternfly/patternfly or any CSS from @patternfly/patternfly. The styles are all basically hard-coded into the chart SVG's.
There was a problem hiding this comment.
Ah nope, I was misunderstanding/assuming that you had to install @patternfly/patternfly in either case! This is ringing a bell for me now, ty
|
|
||
| Default styles in the [@patternfly/react-charts package](https://www.npmjs.com/package/@patternfly/react-charts) are aligned with our light theme. To use chart styles, you must also have the [@patternfly/patternfly package](https://www.npmjs.com/package/@patternfly/patternfly) installed. | ||
|
|
||
| To utilize default chart styles, you don't need to import anything else. But, to support dark-themed charts, you must also import the stylesheet that contains dark theme styles. To do so, add this line before importing your main application component: |
There was a problem hiding this comment.
Also confused about "utilize default chart styles" - not sure what "utilize" means exactly. Same with "to support dark-themed charts" - not sure what "support" means. If I simplifed it, I'd say charts works with light theme out of the box, no additional styles or anything are needed. However, for charts to work in dark theme, you'll need to import 1) patternfly CSS (so they have our global tokens) and 2) @patternfly/patternfly/patternfly-charts.css
There was a problem hiding this comment.
Just to clarify, when you say "global tokens", do you mean design tokens? Or this file? Which, this file is ultimately our design tokens..right?
I should find time to create a cheat sheet for some of these terms React tokens, design tokens, variables, styles, etc. I get so mixed up 😅
There was a problem hiding this comment.
Your simplified suggestion makes sense outside of me making sure I understand the verbiage, though! Gonna make updates based off this
|
|
||
| `import '@patternfly/patternfly/patternfly-charts.css';` | ||
|
|
||
| Once you import this file, you'll have access to [all chart variables](https://www.npmjs.com/package/@patternfly/patternfly?activeTab=code). Beyond dark theme, you could use these variables to match the style of other UI elements to your chart styles. |
There was a problem hiding this comment.
IMO it isn't really intuitive that "all chart variables" links to @patternfly/patternfly on NPM. I would probably be confused clicking on that, expecting it to take me to a list of chart variables or something like that. I might pair this sentence with the one below about filtering the all tokens table, but it's worth noting that chart variables and chart tokens aren't exactly the same.
There was a problem hiding this comment.
Okay this is great feedback! I was thinking that the patternfly-charts.css file over on NPM was the chart variables, but I'm definitely treading water with variables, tokens, and friends lol
There was a problem hiding this comment.
I think my remaining question is, where is a list of our chart variables?
mcoker
left a comment
mcoker
left a comment
There was a problem hiding this comment.
Left a question, but I'm also good with this as it is! I know this PR has been open a while so I don't want to hold it up any more than I already have 😅
| Default styles in the [@patternfly/react-charts package](https://www.npmjs.com/package/@patternfly/react-charts) are aligned with our light theme. Charts work with PatternFly's light theme by default—you don't need to import anything else. | ||
|
|
||
| To support dark-themed charts, you must: | ||
| 1. Import the [@patternfly/patternfly package](https://www.npmjs.com/package/@patternfly/patternfly), so that you can use our global tokens. |
There was a problem hiding this comment.
Are we wanting to provide documentation for an app that is not using PatternFly? Or are we assuming the charts will be included in a PatternFly app? If they aren't using PatternFly already, they'll need to import some other things.
4 participants
Closes #2974
As a follow up to this PR, I'll need to remove redundant info that I copied from the dark theme handbook.