Skip to content

fix(Wizard): update close button and title structures #9041

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
May 17, 2023
Merged

fix(Wizard): update close button and title structures #9041

merged 5 commits into from
May 17, 2023

Conversation

@nicolethoen
Copy link
Contributor

@nicolethoen nicolethoen commented May 4, 2023

What: Closes #8254

Updates the elements wrapping the wizard close button and wizard title to match the core changes.

@nicolethoen nicolethoen requested a review from thatblindgeye May 4, 2023 02:06
@patternfly-build
Copy link
Contributor

patternfly-build commented May 4, 2023 •
edited
Loading

Preview: https://patternfly-react-pr-9041.surge.sh

A11y report: https://patternfly-react-pr-9041-a11y.surge.sh

@tlabaj tlabaj requested review from mcoker and srambach May 4, 2023 20:59
@nicolethoen nicolethoen force-pushed the wizard_element_updates branch from 14e6c14 to fc14c8a Compare May 5, 2023 19:46
@srambach
Copy link
Member

srambach commented May 5, 2023

The "In page" and "In modal" examples are giving an error "TypeError: w is not a function"

Also, I notice that the wizard description is a <p> although core uses a <div> and after chatting, you found this which I will put here for reference! Looks like we meant to change this to a div at a breaking change. #7122 (comment)

@nicolethoen
Copy link
Contributor Author

nicolethoen commented May 5, 2023

broken full page demos should be fixed when this PR merges.

thatblindgeye
thatblindgeye reviewed May 11, 2023
View reviewed changes
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.

Looks good, only issue is the typeerror on those two demos Sarah mentioned above is persisting. Wondering whether the update made in #9093 will help with that.

wise-king-sullyman
wise-king-sullyman approved these changes May 11, 2023
View reviewed changes
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.

Everything looks good to me other than the already mentioned error. I tested locally and the change to DashboardWrapper I made in #9093 does resolve it.

@nicolethoen nicolethoen force-pushed the wizard_element_updates branch from 87ab0f8 to 4420e31 Compare May 12, 2023 02:39
srambach
srambach approved these changes May 15, 2023
View reviewed changes
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.

Looks good to me 👍

@tlabaj tlabaj merged commit 2ac733b into patternfly:v5 May 17, 2023
@patternfly-build
Copy link
Contributor

patternfly-build commented May 17, 2023

Your changes have been released in:

  • @patternfly/react-code-editor@5.0.0-alpha.105
  • @patternfly/react-core@5.0.0-alpha.104
  • @patternfly/react-docs@6.0.0-alpha.112
  • demo-app-ts@5.0.0-alpha.88
  • @patternfly/react-integration@5.0.0-alpha.45
  • @patternfly/react-table@5.0.0-alpha.106

Thanks for your contribution! 🎉

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

Reviewers

@wise-king-sullyman wise-king-sullyman wise-king-sullyman approved these changes

@srambach srambach srambach approved these changes

@kmcfaul kmcfaul kmcfaul approved these changes

@thatblindgeye thatblindgeye thatblindgeye approved these changes

@mcoker mcoker Awaiting requested review from mcoker

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Wizard component element updates

7 participants

@nicolethoen @patternfly-build @srambach @wise-king-sullyman @kmcfaul @thatblindgeye @tlabaj