-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[docs] Revise the Charts Custom Components doc #19793
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[docs] Revise the Charts Custom Components doc #19793
Conversation
|
Deploy preview: https://deploy-preview-19793--material-ui-x.netlify.app/ Updated pages:
Bundle size report
|
CodSpeed Performance ReportMerging #19793 will not alter performanceComparing Summary
|
alexfauquette
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took advantage of this PR to spot few sentences we forgot to updat efrom one major to another 🫣
Co-authored-by: Alexandre Fauquette <[email protected]> Signed-off-by: mapache-salvaje <[email protected]>
|
|
||
| - `height` and `width` for the `<svg />` size. If not provided, those values are derived from the container. | ||
| - `margin` for adding space between the `<svg />` border and the **drawing area**. | ||
| - `height` and `width` for the SVG size; if not provided, these values are derived from the container |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe easier by avoiding details
| - `height` and `width` for the SVG size; if not provided, these values are derived from the container | |
| - `height` and `width` for the SVG size |
|
|
||
| Series information is accessible through the `useSeries` hook for all series types, and `useXxxSeries` hook for a specific series type. | ||
| These hooks return the order of the series and their configuration, including data points, color, among others. | ||
| Series information is accessible through the `useSeries()` hook for all series types, and the `useXxxSeries()` hook for a specific series type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mui/xcharts I believe we started using use*Series from the discussion here: #19334 (comment)
| Series information is accessible through the `useSeries()` hook for all series types, and the `useXxxSeries()` hook for a specific series type. | |
| Series information is accessible through the `useSeries()` hook for all series types, and the `use*Series()` hook for a specific series type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The page below adds context to what this is though, in that use-case at least 🤔
In the current use case we might want to add like useBarSeries for example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's maybe cultural, but for me stars are for stuff like sx, f**k, sht, ...
I would prefer one of those options.
use***Series()to specify "there are multiple characters missing"useXxxSeries()to specify "there are a caml case word"use[Type]Series()to specify the mining of the missing world
Whatever the one we pick, we should modify
| To display data, you have components named `<XxxPlot />` such as `<LinePlot />`, `<AreaPlot />`, `<MarkPlot />`, `<BarPlot />`, etc. |
| ## Stabilize `useSeries` and `useXxxSeries` hooks ✅ |
| Series information is accessible through the `useSeries` hook for all series types, and `useXxxSeries` hook for a specific series type. |
| The `use*Series` hooks provide access to specific series data for a particular chart type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use[Type]Series() - I think I like this one best because it seems most obvious to me that the text in brackets needs to be replaced. What about use[Chart]Series() instead? In my mind, that makes it immediately clear to me that I can sub in Bar, Line, etc. there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also fine with adding a parenthetical on first mention: "...the use[Chart]Series() hook (for example, useBarSeries())..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't use[Chart]Series cause confusion? We have a lot of useChart stuff 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point - let's go with use[Type]Series() then
Co-authored-by: Alexandre Fauquette <[email protected]> Signed-off-by: mapache-salvaje <[email protected]>
|
This pull request has been inactive for 30 days. Please remove the stale label or leave a comment to keep it open. Otherwise, it will be closed in 15 days. |
alexfauquette
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JCQuintas Can I let you check the few remaining suggestion I left.
Other than that it looks good to me.
I updated the page to folow the new naming convention Component instead of <Component> or <Component />
|
Thanks for reviving this @alexfauquette ! I just did one final pass and I'm happy with it now. 👍 |
Signed-off-by: mapache-salvaje <[email protected]> Co-authored-by: Alexandre Fauquette <[email protected]> Co-authored-by: alex <[email protected]>
No description provided.