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 in the API page links #36419

Merged
merged 1 commit into from
Mar 6, 2023

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Mar 3, 2023

This is a quick clean-up to fix https://app.ahrefs.com/site-audit/3992793/27/data-explorer?columns=pageRating%2Curl%2Ctraffic%2ChttpCode%2Cdepth%2Ccompliant%2CincomingAllLinks%2Corigin&filterId=91013c8aafad4fe1f7a0ad504eccad42&issueId=c64da643-d0f4-11e7-8ed1-001e67ed4656&sorting=-pageRating after we added the API pages in: #36008. Fixes like my PR are expected, it's 100% normal to have edge cases.

Along the way, I noticed:

  1. that we have markdown files with names that don't match the final URLs. I think that it would be great to keep the two in sync. I renamed the file tables/tables.md -> table/table.md, as one step in this direction. Ideally, we could have a very simple mapping, or even use the markdown as the source of truth for the final page pathname.
  2. the text field docs is missing: As a developer where do I learn how to use an input with a label and a helper text? As far as I could browse the docs from https://mui.com/joy-ui/react-text-field/, it seems missing.

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work docs Improvements or additions to the documentation package: joy-ui Specific to @mui/joy labels Mar 3, 2023
@oliviertassinari oliviertassinari requested a review from hbjORbj March 3, 2023 18:28
@oliviertassinari oliviertassinari marked this pull request as ready for review March 3, 2023 18:28
Copy link
Member

@hbjORbj hbjORbj left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for catching this!

@hbjORbj
Copy link
Member

hbjORbj commented Mar 6, 2023

the text field docs is missing: As a developer where do I learn how to use an input with a label and a helper text? As far as I could browse the docs from https://mui.com/joy-ui/react-text-field/, it seems missing.

(1) Are you saying the API doc for TextField is missing?

(2) We decided to remove TextField from Joy UI as it can be replaced by a combination of FormControl, FormLabel, FormHelperText and Input. Do you expect the component doc for TextField to be still there? or both the component doc and the API doc?

@oliviertassinari oliviertassinari merged commit 07c3518 into mui:master Mar 6, 2023
@oliviertassinari oliviertassinari deleted the fix-docs-404 branch March 6, 2023 22:38
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Mar 6, 2023

  1. The point I want to get to is that the fix in this PR surfaces the complexity of the replaceComponentLinks() logic. We could make this logic significantly simpler with a simple naming convention on the markdown files.
  2. The problem for me is that we don't seem to have docs on how to combine these components. The API reference page for <TextField> that is gone looks expected. I'm haven't thought enough about it to lean on a specific solution. Now, the TextField component docs used to cover how to solve these problems, the API changed, maybe it still make sense to have a TextField docs page using the new lower level APIs.

@hbjORbj
Copy link
Member

hbjORbj commented Mar 7, 2023

  1. Yes, good point. https://github.com/mui/material-ui/blob/master/docs/pages/blog/making-customizable-components.md#drawback-example shows that it's easy to get confused as well. Will talk to Jun about it.

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 package: joy-ui Specific to @mui/joy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants