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

docs(Page): add example showing different type prop variants #10352

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

adamviktora
Copy link
Contributor

@adamviktora adamviktora commented May 10, 2024

What: Closes #9868

Additional issues:

  • Section with type="nav" for tertiary navigation: the tertiary navigation was removed in V6, we should rename it in the upcoming React example for V6 and the Core V6 example too - but what is the type="nav" used for in V6 if not tertiary navigation?

@adamviktora adamviktora requested review from a team, wise-king-sullyman and nicolethoen and removed request for a team May 10, 2024 10:31
@patternfly-build
Copy link
Contributor

patternfly-build commented May 10, 2024

Copy link
Contributor

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

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

Just to note: it seems like our preview builds are having issues, I didn't see this example on the preview but I did see it locally. That local build did show some compilation warnings, but I'm guessing those aren't related to this PR.

It looks like the example is missing the -group section that exists in the core version, but I see that seems to be missing as an option of PageSection so that could be done under a different issue IMO.

Other than wanting that addressed or a followup created and issues brought up in the PR body LGTM, happy to approve once those are resolved.

@wise-king-sullyman wise-king-sullyman requested review from a team and mattnolting and removed request for a team May 13, 2024 19:50
@adamviktora
Copy link
Contributor Author

Yeah, the example is missing in the preview for some reason. In all the new branches I base on current main I see this compilation warning related to the new Dropdown template:

Screenshot 2024-05-14 at 8 56 28

I don't know if it is related to why the example is not showing in the preview though.

About the -group section, as you say, it is not a type prop option of the PageSection, but a PageGroup component, and there is an example already for that. So I did not include the PageGroup there on purpose.

Copy link
Contributor

@nicolethoen nicolethoen left a comment

Choose a reason for hiding this comment

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

Locally, this looks good to me. I'm investigating the pr-preview situation 👍🏻

@jonkoops
Copy link
Contributor

#10416 should fix this problem, let's do a rebase of this PR to see if it resolves the issue.

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

Successfully merging this pull request may close these issues.

Page - add example showing different type prop variants
5 participants