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

Fix LocalesMenuButton's custom AppBar example and polyglotI18nProvider documentation #8452

Merged
merged 8 commits into from
Dec 14, 2022
Merged

Fix LocalesMenuButton's custom AppBar example and polyglotI18nProvider documentation #8452

merged 8 commits into from
Dec 14, 2022

Conversation

smeng9
Copy link
Contributor

@smeng9 smeng9 commented Nov 30, 2022

No description provided.

@fzaninotto
Copy link
Member

I don't think that's the right fix. We should start by explaining that it's inserted automatically if the i18nProvider is properly configured, then explain the manual insertion for custom appbars.

@smeng9
Copy link
Contributor Author

smeng9 commented Dec 1, 2022 •
edited
Loading

I have applied the review to explain first on when this component should be used.

It should now fix the incorrect order of parameters passed to polyglotI18nProvider, and also address duplicate LocalesMenuButton showing up in the AppBar if we want to customize the AppBar.

@smeng9 smeng9 changed the title Fix LocalesMenuButton Documentation causing duplicate LocalesMenuButton to be shown Fix LocalesMenuButton documentation example causing duplicate LocalesMenuButton to be shown Dec 1, 2022
@smeng9
Copy link
Contributor Author

smeng9 commented Dec 6, 2022

@fzaninotto Re-request review

slax57
slax57 requested changes Dec 8, 2022
View reviewed changes
docs/LocalesMenuButton.md Outdated Show resolved Hide resolved
docs/LocalesMenuButton.md Outdated Show resolved Hide resolved
docs/LocalesMenuButton.md Outdated Show resolved Hide resolved
smeng9 and others added 2 commits December 8, 2022 11:53
@smeng9 @slax57
Update docs/LocalesMenuButton.md
1b7d3e1 
Co-authored-by: Jean-Baptiste Kaiser <jbaptiste.kaiser@gmail.com>
@smeng9 @slax57
Update docs/LocalesMenuButton.md
61446f0 
Co-authored-by: Jean-Baptiste Kaiser <jbaptiste.kaiser@gmail.com>
@smeng9 smeng9 requested a review from slax57 December 12, 2022 07:27
@smeng9 smeng9 changed the title Fix LocalesMenuButton documentation example causing duplicate LocalesMenuButton to be shown Fix LocalesMenuButton's custom AppBar example and polyglotI18nProvider documentation Dec 14, 2022
slax57
slax57 requested changes Dec 14, 2022
View reviewed changes
Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

One typo remaining (I'm not sure about the suggested fix so I'll let you review it @smeng9 ), otherwise this seems good to me, thanks!

docs/LocalesMenuButton.md Outdated Show resolved Hide resolved
@smeng9 @slax57
Update docs/LocalesMenuButton.md
8704195 
Co-authored-by: Jean-Baptiste Kaiser <jbaptiste.kaiser@gmail.com>
slax57
slax57 approved these changes Dec 14, 2022
View reviewed changes
Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

Thanks!

@slax57 slax57 modified the milestones: 4.7.0, 4.6.2 Dec 14, 2022
@slax57 slax57 merged commit d0438b7 into marmelab:master Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slax57 slax57 slax57 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
4.6.2
Development

Successfully merging this pull request may close these issues.
3 participants
@smeng9 @fzaninotto @slax57