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.

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

feat(select next): add appendTo #9562

Merged
merged 1 commit into from
Sep 21, 2023
Merged

feat(select next): add appendTo #9562

merged 1 commit into from
Sep 21, 2023

Conversation

MariaAga
Copy link
Contributor

@MariaAga MariaAga commented Sep 1, 2023

What: Closes #9557

@patternfly-build
Copy link
Contributor

patternfly-build commented Sep 1, 2023 •
edited
Loading

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

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

thatblindgeye
thatblindgeye reviewed Sep 1, 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 nit is possibly reducing duplication of the Popper element being returned in the ternary since only the wrapper div and appendTo prop are different, not a blocker though.

@tlabaj should we just go ahead and make a similar update for dropdown next in v4? I thought the original issue was for the dropdown, though select was also discussed during office hours.

@wise-king-sullyman
Copy link
Contributor

I made #9564 for the Dropdown next fix.

wise-king-sullyman
wise-king-sullyman approved these changes Sep 1, 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.

Agree with @thatblindgeye about the duplication, other than that LGTM.

@MariaAga MariaAga force-pushed the seelct-next-append-to branch from b858542 to 543439c Compare September 4, 2023 10:03
@MariaAga
Copy link
Contributor Author

MariaAga commented Sep 4, 2023

thanks, removed duplicates

@tlabaj tlabaj merged commit 9340381 into patternfly:v4 Sep 21, 2023
10 checks passed
@patternfly-build
Copy link
Contributor

Your changes have been released in:

  • @patternfly/react-code-editor@4.82.120
  • @patternfly/react-core@4.277.0
  • @patternfly/react-docs@5.103.88
  • @patternfly/react-inline-edit-extension@4.86.127
  • demo-app-ts@4.210.17
  • @patternfly/react-table@4.113.5
  • @patternfly/react-virtualized-extension@4.88.120

Thanks for your contribution! 🎉

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

@thatblindgeye thatblindgeye thatblindgeye approved these changes

@kmcfaul kmcfaul kmcfaul approved these changes

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

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@MariaAga @patternfly-build @wise-king-sullyman @kmcfaul @thatblindgeye @tlabaj