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(Wizard): use height token to set height #10332

Merged
merged 3 commits into from
May 28, 2024

Conversation

kmcfaul
Copy link
Contributor

@kmcfaul kmcfaul commented May 2, 2024

What: React counterpart to patternfly/patternfly#6457

@patternfly-build
Copy link
Contributor

patternfly-build commented May 2, 2024

Copy link
Contributor

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

Doesn't look like passing just a number works anymore, at least based on the basic Wizard example. We should probably update the height prop type to just a string, and update any instance of it being used to pass a string/CSS unit value.

The Wizard height doesn't seem to be adjustable inside a modal still, either. @mcoker should React set the --#{$modal-box}--c-wizard--FlexBasis token as well?

@kmcfaul
Copy link
Contributor Author

kmcfaul commented May 8, 2024

I can update the prop type to string (or fix it so numbers work).

The core counterpart to this isn't merged yet, so it's not setting the flex-basis with the value atm.

@kmcfaul
Copy link
Contributor Author

kmcfaul commented May 8, 2024

Pending further updates to patternfly/patternfly#6457, this PR will need to updated to use the new --#{$wizard}--height token once core is updated in react. Update: token will be the same but this PR will still need the core bump first.

@kmcfaul kmcfaul linked an issue May 15, 2024 that may be closed by this pull request
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.

Other than updating/fixing the width prop type so numbers don't break, lgtm! Agree it should be good once this is updated with the latest core.

@tlabaj tlabaj merged commit 2b5a598 into patternfly:v6 May 28, 2024
13 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.

Wizard - setting height does not work when inside of a modal Bug - Wizard - cannot customize height in modal
5 participants