-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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][charts] Fix anchorEl API page #15625
[docs][charts] Fix anchorEl API page #15625
Conversation
4421f9f
to
a6bba1d
Compare
Deploy preview: https://deploy-preview-15625--material-ui-x.netlify.app/ |
CodSpeed Performance ReportMerging #15625 will improve performances by 36.63%Comparing Summary
Benchmarks breakdown
|
The performance benchmark run in dev mode, shouldn't we move this to be with https://codspeed.io/mui/mui-x/branches/oliviertassinari:chart-fix-anchor-el-docs Oh wait, it doesn't run in a real browser but in jsdom. Hum, OK, results won't be super helpful. |
It runs on our built version, but I suppose I didn't make the jsdom run using production mode 🤔 But indeed, the goal is mainly to see in case we make mistakes or improvements. They are not meant to represent "how fast the charts are", but "how versions speeds compare" |
a6bba1d
to
f411196
Compare
@JCQuintas Right, yeah, we will get some directional indicators from those benchmark assertions 👍. However, we can't base merge or tradeoff decisions based on them. A change could be slower in jsdom but faster in a real browser, it could be slower in dev mode, but faster in prod mod. |
Fix #15575 (comment). It's a quick win, so why not. The solution is to copy https://mui.com/material-ui/api/popper/#popper-prop-anchorEl.
Before: https://master--material-ui-x.netlify.app/x/api/charts/charts-tooltip/#props
After. https://deploy-preview-15625--material-ui-x.netlify.app/x/api/charts/charts-tooltip/#props