-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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] Fix 404 links #15575
[docs] Fix 404 links #15575
Conversation
Deploy preview: https://deploy-preview-15575--material-ui-x.netlify.app/ Updated pages: |
/** | ||
* @ignore - internal component. | ||
*/ |
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.
Even if it's applied to a different component, the shouldSkip
logic triggers before we ever try to read the jsdocs of the components (to be faster), so we have to remove this. to get the API page of HeatmapTooltip
to generate.
We missed this in #15154
function TreeItemProvider(props: TreeItemProviderProps) { | ||
const { children, itemId, id } = props; | ||
const { wrapItem, instance, store } = useTreeViewContext<[]>(); | ||
const treeId = useSelector(store, selectorTreeViewId); | ||
const idAttribute = generateTreeItemIdAttribute({ itemId, treeId, id }); | ||
|
||
return wrapItem({ children, itemId, instance, idAttribute }); | ||
return <React.Fragment>{wrapItem({ children, itemId, instance, idAttribute })}</React.Fragment>; |
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.
We can remove this once mui/material-ui#44516 lands.
We missed this in #14913
@@ -63,321 +63,4 @@ function ChartDataProviderPro(props: ChartDataProviderProProps) { | |||
); | |||
} | |||
|
|||
ChartDataProviderPro.propTypes = { |
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.
This is dead code, ChartDataProviderPro
is a private API so shouldn't have prop types. It also happens to be wrong, go to https://next.mui.com/x/react-charts/heatmap/ and you will get a runtime warning with the width
prop required.
We missed this in #15375.
```diff | ||
- import { usePickersTranslations } from '@mui/x-date-pickers/hooks'; | ||
- import { usePickersTranslations } from '@mui/x-date-pickers'; | ||
- import { usePickersTranslations } from '@mui/x-date-pickers-pro'; | ||
-import { usePickersTranslations } from '@mui/x-date-pickers/hooks'; | ||
-import { usePickersTranslations } from '@mui/x-date-pickers'; | ||
-import { usePickersTranslations } from '@mui/x-date-pickers-pro'; |
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.
Fix git diff format
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.
Is there a way to lint such cases to avoid introducing incorrect diffs in the first place? 🤔
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.
It's possible yes. I did a first pass a long time ago https://github.com/mui/material-ui/blob/master/packages/markdownlint-rule-mui/git-diff.js. We could do another, I didn't think about this case.
We can simply make sure that each diff resets the spacing, in a way that there is is no visual spacing offsets, e.g.
- foo() {}
- bar;
+foo() {}
+ bar;
@samuelsycamore if you are looking for some small writing code + some algorithmic exercises, up for grab 😄. I can allocate time to review it.
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.
Moved to an issue mui/mui-public#249.
components: Heatmap, HeatmapPlot, DefaultHeatmapTooltip | ||
components: Heatmap, HeatmapPlot, HeatmapTooltip |
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 correct name
2c06cf3
to
942840a
Compare
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.
Thank you for taking the time on these changes! 🙏
LGTM, I just see 2 open question for charts and tree view teams. 🤔
```diff | ||
- import { usePickersTranslations } from '@mui/x-date-pickers/hooks'; | ||
- import { usePickersTranslations } from '@mui/x-date-pickers'; | ||
- import { usePickersTranslations } from '@mui/x-date-pickers-pro'; | ||
-import { usePickersTranslations } from '@mui/x-date-pickers/hooks'; | ||
-import { usePickersTranslations } from '@mui/x-date-pickers'; | ||
-import { usePickersTranslations } from '@mui/x-date-pickers-pro'; |
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.
Is there a way to lint such cases to avoid introducing incorrect diffs in the first place? 🤔
942840a
to
034674d
Compare
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.
Thank you for all the refinement work! 👍 🎉
Diving a bit, a bit too deep actually, but 😁
The new API pages: