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 broken Next and Previous links #29711

Merged
merged 2 commits into from
Nov 23, 2021

Conversation

scallaway
Copy link
Contributor

@scallaway scallaway commented Nov 16, 2021

I still don't quite understand why this was caused, since the changes have been in master for a few days and I've been able to access the MUI docs in the interim (I think, although can't be fully sure since we haven't migrated from 4 to 5 yet), but I guess it's due to the way that you guys do releases?

I also have a feeling that the typing for the props expected to be passed to the Link component in docs/src/modules/components/Link.tsx are slightly wrong since we should be Omiting the linkAs prop as specified in the typing.

Happy to have more of a look at this in the future, but this at least un-blocks the prod website from being completely broken.

Fixes #29704
Fixes #29708

https://deploy-preview-29711--material-ui.netlify.app/api/form-control-label/

@mui-pr-bot
Copy link

mui-pr-bot commented Nov 16, 2021

No bundle size changes

Generated by 🚫 dangerJS against 77c596f

@scallaway
Copy link
Contributor Author

The build failure looks unrelated to my changes 🤔

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work docs Improvements or additions to the documentation regression A bug, but worse labels Nov 16, 2021
@oliviertassinari
Copy link
Member

@scallaway The linkAs prop was not caught by the locale logic. I have updated a bit the approach to retain the fixes I did in #29624.

@scallaway
Copy link
Contributor Author

I guess this is the reason why some of the links in prod were working but others weren't?

I did have a feeling it had something to locale or the like, since the lookup within the page title functionality wasn't working properly.

It didn't really help that the errors thrown by React weren't that useful, so that could be something to be improved upon in the future? Maybe that's also just my unfamiliarity with the codebase, but surely you'd want that to be as welcoming to new devs as possible!

Your changes do look a lot more robust than mine!

return { ...page, linkProps: { as: `${page.pathname.replace(/^\/api-docs/, '/api')}/` } };
return {
...page,
linkProps: { linkAs: `${page.pathname.replace(/^\/api-docs/, '/api')}/` },
Copy link
Member

Choose a reason for hiding this comment

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

@mui-org/x heads up, this is a breaking change, you need to apply the same diff once you upgrade the mono-repo. We improve the support for the translation of the docs.

@michaldudak michaldudak merged commit 42f425a into mui:master Nov 23, 2021
@siriwatknp siriwatknp mentioned this pull request Dec 14, 2021
1 task
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.

[docs] update import path of unstyled button in docs Client-side exception at mui.com/api/grid/
5 participants