Skip to content
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

[charts-pro] Release the pro package in alpha #13859

Merged
merged 21 commits into from
Aug 5, 2024

Conversation

alexfauquette
Copy link
Member

@alexfauquette alexfauquette commented Jul 16, 2024

Changelog

  • 🎨 Charts get a new component to display color mapping in the legend.
    image

  • 🚀 The @mui/x-charts-pro is released in alpha version 🧪. This new package introduces two main features:

    • The Heatmap component
    • The zoom interaction on the bar, line, and scatter charts

Preview: https://deploy-preview-13859--material-ui-x.netlify.app/x/react-charts/heatmap/

@alexfauquette alexfauquette added the component: charts This is the name of the generic UI component, not the React module! label Jul 16, 2024
@alexfauquette alexfauquette marked this pull request as ready for review July 17, 2024 11:36
@oliviertassinari oliviertassinari added the plan: Pro Impact at least one Pro user label Jul 17, 2024
@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 17, 2024

Cool to see near its release.

All the things that I can spot on https://deploy-preview-13859--material-ui-x.netlify.app/x/react-charts/heatmap/:

  1. import { UnstableHeatmap } from '@mui/x-charts-pro/Heatmap'; is usually import { Unstable_Heatmap } from '@mui/x-charts-pro/Heatmap';
  2. import '@mui/x-charts-pro/typeOverloads'; in the demo seems strange to me. I would expect this to be in the npm package
  3. Should https://deploy-preview-13859--material-ui-x.netlify.app/x/react-charts/zoom-and-pan/ also have the "Preview" badge? It's released as 7.0.0-alpha.0
  4. https://deploy-preview-13859--material-ui-x.netlify.app/x/introduction/licensing/#pro-plan should also list the charts?
  5. The JS file isn't converted
SCR-20240718-bcxn
  1. The popover might be able to get too close to the edge of the screen:
SCR-20240718-bdvv
  1. https://deploy-preview-13859--material-ui-x.netlify.app/x/react-charts/zoom-and-pan/#zooming-options Sentence case
  2. I'm not sold on the color contrast in dark mode https://deploy-preview-13859--material-ui-x.netlify.app/x/react-charts/zoom-and-pan/#zooming-options, both series looks hard to separate.
SCR-20240718-beyk
  1. The zoom feature is very cool. However, I'm confused about the UX. Moved to [charts][zoom] Allow to zoom on data #12503 (comment).

  2. Ads should be disabled on https://deploy-preview-13859--material-ui-x.netlify.app/x/react-charts/heatmap/, https://deploy-preview-13859--material-ui-x.netlify.app/x/react-charts/zoom-and-pan/

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 18, 2024
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 18, 2024
@alexfauquette
Copy link
Member Author

alexfauquette commented Jul 18, 2024

  1. Done. The Unstable is useless since we are in alpha

  2. Using the import '@mui/x-charts-pro/typeOverloads'; in the index.ts is not enough. To fix TS, I needed to add it to the Heatmap/index.ts. I don't know where else to put that

  3. Added "Preview" badge to zoom and pan link

  4. I added charts to the licencing page. @joserodolfofreitas should we rewrite some sections?

  5. The JS file isn't converted -> can't do a lot about that. It's related to [docs-infra] Multi-tab demo component improvements material-ui#41754

  6. The popover might be able to get too close to the edge of the screen. -> I don't get the point? The issue I see is the left axis not getting enough space, but no problem with the popover

  7. https://deploy-preview-13859--material-ui-x.netlify.app/x/react-charts/zoom-and-pan/#zooming-options Sentence case -> I assume it's the "O" of "Zooming Options"

  8. I'm not sold on the color contrast in dark mode https://deploy-preview-13859--material-ui-x.netlify.app/x/react-charts/zoom-and-pan/#zooming-options, both series looks hard to separate. -> We can change them, but in a major since it's the default colors of the charts

  9. The zoom feature is very cool. However, I'm confused about the UX. Moved to [charts][zoom] Allow to zoom on data #12503 (comment).

  10. Ads have been disabled

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 18, 2024

Thanks for the updates.

  1. The popover might be able to get too close to the edge of the screen. -> I don't get the point? The issue I see is the left axis not getting enough space, but no problem with the popover

I have this in mind mui/base-ui#39. But maybe we can't fix this yet.


Per https://mui-org.slack.com/archives/C02EQ97NV8F/p1721308954099199, I suspect that we would want to merge this PR but keep "private": true in the package.json since those pages are already live, e.g. https://mui.com/x/react-charts/heatmap/.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 19, 2024
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 2, 2024
Copy link

codspeed-hq bot commented Aug 2, 2024

CodSpeed Performance Report

Merging #13859 will not alter performance

Comparing alexfauquette:prepare-pro-release (8b9ed6a) with master (d54212d)

Summary

✅ 3 untouched benchmarks

@alexfauquette alexfauquette merged commit 2c54b97 into mui:master Aug 5, 2024
19 checks passed
@alexfauquette alexfauquette deleted the prepare-pro-release branch August 5, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: charts This is the name of the generic UI component, not the React module! plan: Pro Impact at least one Pro user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants