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] Prepare ChartContainerPro for future Zoom changes #13532

Merged
merged 9 commits into from
Jun 19, 2024

Conversation

JCQuintas
Copy link
Member

  • This shouldn't change any of the current behaviours
  • Add ChartContainerPro for Zoom changes

related #13405

@JCQuintas JCQuintas added enhancement This is not a bug, nor a new feature component: charts This is the name of the generic UI component, not the React module! labels Jun 18, 2024
@mui-bot
Copy link

mui-bot commented Jun 18, 2024

Deploy preview: https://deploy-preview-13532--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against cb0fa50

@JCQuintas JCQuintas force-pushed the prepare-chart-container-pro branch from 522d4d9 to c5595c0 Compare June 18, 2024 14:58
@@ -17,12 +17,10 @@ const ResponsiveChartContainerPro = React.forwardRef(function ResponsiveChartCon
const { width: inWidth, height: inHeight, ...other } = props;
const [containerRef, width, height] = useChartContainerDimensions(inWidth, inHeight);

useLicenseVerifier('x-charts-pro', releaseInfo);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could effectively be moved in the the ChartContainerPro, but I do not see where you added it back

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops. I was tinkering with the license things 😅

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though the Watermark component runs it already, would we still need it?

export function Watermark(props: WatermarkProps) {
  const { packageName, releaseInfo } = props;
  const licenseStatus = useLicenseVerifier(packageName, releaseInfo);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the Watermark component runs it already

I did not know

would we still need it

Could be nice in the container since it does not have Watermark, and adding the watermark could add complication

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@JCQuintas JCQuintas merged commit 83b2695 into mui:master Jun 19, 2024
17 checks passed
@JCQuintas JCQuintas deleted the prepare-chart-container-pro branch June 19, 2024 15:55
DungTiger pushed a commit to DungTiger/mui-x that referenced this pull request Jul 23, 2024
Signed-off-by: Jose C Quintas Jr <[email protected]>
Co-authored-by: Flavien DELANGLE <[email protected]>
thomasmoon pushed a commit to thomasmoon/mui-x that referenced this pull request Sep 9, 2024
Signed-off-by: Jose C Quintas Jr <[email protected]>
Co-authored-by: Flavien DELANGLE <[email protected]>
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! enhancement This is not a bug, nor a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants