-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Fix typos in Theming doc #5546
Fix typos in Theming doc #5546
Conversation
- typo - no error page in example, so I removed the mention in the text - I added a comment in the text itself, another occurrence of something mentioned in text that doesn't appear in code. So I guess you have some fixing to do before merging.
docs/Theming.md
Outdated
@@ -387,7 +387,7 @@ const App = () => ( | |||
); | |||
``` | |||
|
|||
Your custom layout can extend the default `<Layout>` component if you only want to override the sidebar, the appBar, the menu, the notification component, or the error page. For instance: | |||
Your custom layout can extend the default `<Layout>` component if you only want to override the sidebar, the appBar, the menu or the notification component. For instance: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though the example doesn't show how to change the error page, the layout has this role, so you should revert that change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 63c2d5f
docs/Theming.md
Outdated
@@ -890,7 +890,7 @@ const App = () => ( | |||
|
|||
The `MenuItemLink` component make use of the React Router [NavLink](https://reacttraining.com/react-router/web/api/NavLink) component, hence allowing to customize its style when it targets the current page. | |||
|
|||
If the default active style does not suit your tastes, you can override it by passing your own `classes`: | |||
If the default active style does not suit your tastes, you can override it by passing your own `classes`: // the code below doesn't use the `classes` prop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code below, introduced in 8fcd609, doesn't teach anything about classes and doesn't show anything more than the previous snippet. It and should be removed, together with the entire sentence "if the default active style...".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 07e7ed2
Thanks! |
So I guess you have some fixing to do before merging.