-
Notifications
You must be signed in to change notification settings - Fork 375
Move items in main nav #8828
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
Move items in main nav #8828
Conversation
|
Preview: https://patternfly-react-pr-8828.surge.sh A11y report: https://patternfly-react-pr-8828-a11y.surge.sh |
|
@nicolethoen @tlabaj @evwilkin these changes in the React repo are now ready for review. |
|
@mcarrano it looks like the "File upload - multiple" demo needs to be updated similar to the examples file you've already changed, as it isn't grouped within the new "File upload" subsection Also for @tlabaj - for the components renamed in the navigation (File upload => Simple file upload, File upload - multiple => Multiple file upload) should we also rename the exported components in the code? I ask because this would require follow-up React issues to change the name, along with codemods to update imports & usages for consumers. Or do you think going to "Simple file upload" to view docs for the "File upload" component makes sense - these would be the only 2 component docs to not match the component names in that case. |
I believe we were not going to do that at this time. We were renaming the documentation for clarity, but leaving actual exported component names as they were. @tlabaj was that your understanding? |
Yes, I thought we agreed to leave component names as is. |
|
@tlabaj @nicolethoen thanks! @mcarrano this looks good with your last update to move that demo, it just looks like there are 2 merge conflicts that need to be resolved before we're able to merge. |
|
@evwilkin I believe that the two conflicts are because those components got deprecated after I opened this branch and moved to the /deprecated folder. Does that make sense? How do I fix that? |
evwilkin
left a comment
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.
LGTM
evwilkin
left a comment
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.
LGTM
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.
... Specific changes include the following:
- Group Date and Time related components together in the Components navigation.
Should Calander month be included in with these
- Move menu related components into a group named "Menus"
I only see one tab in Dropdown. I should see the "React Deprecated as well.
|
@tlabaj Good catch regarding Calendar Month. I will move that to Date and Time. The other issue was fixed with my last commit. |
|
@evwilkin @nicolethoen @tlabaj Moved Calendar month to Date & Time. Please give a final review. Thanks. |
|
Should we move |
Calendar month is used in the Date and time picker demos and that is why I called that one out to be moved. The timestamp is really independent of what is currently under the "Date and time picker" Nav item. Given the name of the Nav item, I don't know that I would look for it there. |
tlabaj
left a comment
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.
LGTM
Reorganizes items it the workspace towards improved findability on patternfly.org. Tertiary expandable navigation is now used to group items in the component nav. A new "Patterns" section is introduced to replace Demos. Specific changes include the following:
Closes #8777