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

Templates(TypeaheadSelect): Fix handling of selection and dropdown opening #10847

Merged
merged 1 commit into from
Aug 9, 2024

Conversation

jeff-phillips-18
Copy link
Member

What:
Closes #

Description:
Updates the props for TypeaheadSelect and does not keep selection info in state. Fixes an issue where the menu would incorrectly open on option changes.

@patternfly-build
Copy link
Contributor

patternfly-build commented Aug 8, 2024

@jeff-phillips-18 jeff-phillips-18 changed the title [WIP] Templates(TypeaheadSelect): Fix handling of selection and dropdown opening Templates(TypeaheadSelect): Fix handling of selection and dropdown opening Aug 8, 2024
Copy link
Collaborator

@mturley mturley left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @jeff-phillips-18

Copy link
Contributor

@kmcfaul kmcfaul left a comment

Choose a reason for hiding this comment

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

Overall lgtm

I was a little on the fence about explicitly moving logic out of the template back into the user side (for options, selected states). The initial idea for templates was to require minimal user input, and let a user track the internal state via informational callbacks but not have to be responsible for directly managing it. But that said, typeahead is a pretty complex use case for select with a wide variety of applications and requirements, and a user is probably tracking both states already, so maybe some additional necessary user-side code is inevitable and helpful.

@tlabaj tlabaj merged commit 09b1682 into patternfly:main Aug 9, 2024
13 checks passed
@patternfly-build
Copy link
Contributor

Your changes have been released in:

Thanks for your contribution! 🎉

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.

5 participants