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

[docs] Fix 404 links #15575

Merged
merged 4 commits into from
Nov 27, 2024
Merged

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.

docs/data/tree-view/getting-started/getting-started.md Outdated Show resolved Hide resolved
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 bug 🐛 Something doesn't work regression A bug, but worse 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? 🤔

docs/data/tree-view/getting-started/getting-started.md Outdated Show resolved Hide resolved
docs/pages/x/api/charts/heatmap-tooltip.json Show resolved Hide resolved
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work docs Improvements or additions to the documentation regression A bug, but worse
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants