Skip to content

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Nov 24, 2024

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation. label Nov 24, 2024
Comment on lines -42 to -44
/**
* @ignore - internal component.
*/
Copy link
Member Author

@oliviertassinari oliviertassinari Nov 24, 2024

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>;
Copy link
Member Author

@oliviertassinari oliviertassinari Nov 24, 2024

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 = {
Copy link
Member Author

@oliviertassinari oliviertassinari Nov 24, 2024

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.

Comment on lines 308 to +360
```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';
Copy link
Member Author

@oliviertassinari oliviertassinari Nov 24, 2024

Choose a reason for hiding this comment

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

Fix git diff format

Copy link
Member

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? 🤔

Copy link
Member Author

@oliviertassinari oliviertassinari Nov 25, 2024

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.

Copy link
Member Author

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
Copy link
Member Author

Choose a reason for hiding this comment

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

The correct name

@oliviertassinari oliviertassinari force-pushed the fix-404-links-v2 branch 2 times, most recently from 2c06cf3 to 942840a Compare November 24, 2024 00:43
@oliviertassinari oliviertassinari added type: bug It doesn't behave as expected. type: regression A bug, but worse, it used to behave as expected. labels Nov 24, 2024
Copy link
Member

@LukasTy LukasTy left a 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. 🤔

Comment on lines 308 to +360
```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';
Copy link
Member

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? 🤔

Copy link
Member

@LukasTy LukasTy left a 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! 👍 🎉

@oliviertassinari oliviertassinari merged commit ee6ee3e into mui:master Nov 27, 2024
18 checks passed
@oliviertassinari oliviertassinari deleted the fix-404-links-v2 branch November 27, 2024 09:46
LukasTy pushed a commit to LukasTy/mui-x that referenced this pull request Dec 19, 2024
@oliviertassinari oliviertassinari removed the type: bug It doesn't behave as expected. label Apr 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation. type: regression A bug, but worse, it used to behave as expected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants