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

feat(PageSidebar): added SidebarBody component #8942

Merged
merged 3 commits into from Apr 28, 2023

Conversation

thatblindgeye
Copy link
Contributor

@thatblindgeye thatblindgeye commented Apr 12, 2023

What: Closes #8386

This may require the sideNavLayout in org to be updated again since that is also using PageSidebar (right now the sidebar in the app won't have any content rendered despite being able to open/close it).

Links to where updates should apply (will need to manually update the URL in the address bar to get to any other page):
Page component
Nav component
Notification drawer component
Wizard component
Masthead demos

Additional issues:

@patternfly-build
Copy link
Contributor

patternfly-build commented Apr 12, 2023

Copy link
Contributor

@tlabaj tlabaj 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 to me other than the Org work that needs ti be don to update the nav. Could you please open an issue for that if it does not already exist.

Copy link
Member

@srambach srambach left a comment

Choose a reason for hiding this comment

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

CSS looks good afaict!

Copy link
Contributor

@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

LGTM. I noticed there are some docs around isFilled on <PageSection>s mentioning that the last page section is filled by default, which is a (the?) use case for isFilled={false}. <PageSidebarBody> behaves the same way - not sure if it's worth mentioning?

@thatblindgeye
Copy link
Contributor Author

@mcoker good callout, added some short verbiage regarding that to the "Multiple sidebar body" example

@tlabaj tlabaj merged commit 67827a8 into patternfly:v5 Apr 28, 2023
10 checks passed
@patternfly-build
Copy link
Contributor

Your changes have been released in:

  • @patternfly/react-code-editor@5.0.0-alpha.84
  • @patternfly/react-core@5.0.0-alpha.83
  • @patternfly/react-docs@6.0.0-alpha.90
  • demo-app-ts@5.0.0-alpha.67
  • @patternfly/react-integration@5.0.0-alpha.33
  • @patternfly/react-table@5.0.0-alpha.85

Thanks for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for page sidebar custom content
8 participants