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

Page penta updates #579

Merged
merged 4 commits into from
Feb 28, 2024
Merged

Conversation

Dominik-Petrik
Copy link
Contributor

close #548

Copy link
Collaborator

@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.

One small non-blocking comment, but overall looks great 💪

},
]
});

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like a lot of these files could use a prettier run.

@wise-king-sullyman
Copy link
Collaborator

The build issue here should be fixable after a rebase now

Copy link
Collaborator

@thatblindgeye thatblindgeye left a comment

Choose a reason for hiding this comment

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

Few quick comments below. This can be a followup, but personally I think these 3 rules could be written as a single "pageRenameProps" (or updateProps) rule. May be worth keeping them separated for now and getting opinions from consumer on what they find easier/better, having more rules separated out or having less rules where some cover more than a single change.

@@ -0,0 +1,17 @@
### page-rename-isTertiaryNavGrouped [(#9948)](https://github.com/patternfly/patternfly-react/pull/9948)

We've renamed the \`isTertiaryNavGrouped\` prop to \`isHorizontalSubnavGrouped\`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
We've renamed the \`isTertiaryNavGrouped\` prop to \`isHorizontalSubnavGrouped\`.
We've renamed the `isTertiaryNavGrouped` prop to `isHorizontalSubnavGrouped` on Page.

just to have the props as inline code in the README. We can leave the backticks escaped in the rule/tests, just add the "on Page." at the end of the message in those files as well.

@@ -0,0 +1,17 @@
### page-rename-isTertiaryNavWidthLimited [(#9948)](https://github.com/patternfly/patternfly-react/pull/9948)

We've renamed the \`isTertiaryNavWidthLimited\` prop to \`isHorizontalSubnavWidthLimited\`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similar to comment above.

@@ -0,0 +1,17 @@
### page-rename-tertiaryNav [(#9948)](https://github.com/patternfly/patternfly-react/pull/9948)

We've renamed the \`tertiaryNav\` prop to \`horizontalSubnav\`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similar to above.

@thatblindgeye thatblindgeye merged commit d114454 into patternfly:main Feb 28, 2024
2 checks passed
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 - Penta changes
3 participants