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.

Sign up for GitHub

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

Jump to bottom

[docs] Change ThemeProvider API links #30705

Merged
merged 2 commits into from
Jan 28, 2022
Merged

[docs] Change ThemeProvider API links #30705

merged 2 commits into from
Jan 28, 2022

Conversation

atakanzen
Copy link
Contributor

Former link was redirecting to a 404 page. I presume this link is the correct one, cheers!

@mui-bot
Copy link

mui-bot commented Jan 20, 2022 •
edited
Loading

No bundle size changes

Generated by 🚫 dangerJS against a97afcc

hbjORbj
hbjORbj approved these changes Jan 20, 2022
View reviewed changes
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.

Thanks for spotting this!

@hbjORbj hbjORbj changed the title Change ThemeProvider API links [docs] Change ThemeProvider API links Jan 20, 2022
@hbjORbj hbjORbj added the docs Improvements or additions to the documentation label Jan 20, 2022
mnajdova
mnajdova requested changes Jan 24, 2022
View reviewed changes
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

I propose we duplicate the content for the API on the pages where we document these components. The main motivation is because there are different components. More over, the styles package is legacy, so the pages could very easily be removed in the future.

@mnajdova
Copy link
Member

cc @siriwatknp tagging you to me aware regardless of what the resolution will be

@atakanzen
Copy link
Contributor Author

I propose we duplicate the content for the API on the pages where we document these components. The main motivation is because there are different components. More over, the styles package is legacy, so the pages could very easily be removed in the future.

Just to clear things up, is your sugguestion is to have a dedicated page for ThemeProvider component in the following url /api/theme-provider with the already available information on /styles/api/#themeprovider?

@mnajdova
Copy link
Member

Just to clear things up, is your sugguestion is to have a dedicated page for ThemeProvider component in the following url /api/theme-provider with the already available information on /styles/api/#themeprovider?

I think the best place is to document it under the system, as this is where the component is defined. @siriwatknp should we add an API page here directly in the system, or what do you think would be the best practice, considering the migration. Should we wait and do this after the migration is done?

@mnajdova
Copy link
Member

Maybe we can link these to https://mui.com/customization/theming/#themeprovider

@siriwatknp
Copy link
Member

siriwatknp commented Jan 28, 2022 •
edited
Loading

Maybe we can link these to https://mui.com/customization/theming/#themeprovider

Oh, great. let's fix the 404 by linking to https://mui.com/customization/theming/#themeprovider first and then once the migration is done, we can revisit it again.

atakanzen and others added 2 commits January 28, 2022 05:26
@atakanzen @hbjORbj
Change ThemeProvider API link
e3e5da0 
- Former link was redirecting to a 404 page.
@hbjORbj
Fix links
a97afcc
@hbjORbj hbjORbj force-pushed the api-link-correction branch from 889f2ee to a97afcc Compare January 28, 2022 05:35
@hbjORbj hbjORbj merged commit e048459 into mui:master Jan 28, 2022
wladimirguerra pushed a commit to wladimirguerra/material-ui that referenced this pull request Feb 2, 2022
@atakanzen @wladimirguerra
[docs] Change ThemeProvider API links (mui#30705)
1c2a786
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@siriwatknp siriwatknp siriwatknp left review comments

@mnajdova mnajdova mnajdova requested changes

@hbjORbj hbjORbj hbjORbj approved these changes

Assignees
No one assigned
Labels
docs Improvements or additions to the documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@atakanzen @mui-bot @mnajdova @siriwatknp @hbjORbj