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

[select] fix(Select2): check shouldDismissPopover on menu items #6070

Merged
merged 3 commits into from
Apr 17, 2023

Conversation

PatrickBrown1
Copy link
Contributor

@PatrickBrown1 PatrickBrown1 commented Apr 15, 2023

Fixes #4001

Checklist

  • Includes tests
  • Update documentation
    Don't see a need to update docs because it fixes a feature that should be working.

Changes proposed in this pull request:

Fixes problem where popover was closing on selection despite shouldDismissPopover being set to false on MenuItem2.

Reviewers should focus on:

Ensuring popover open state is updated correctly

Screenshot

N/A

@PatrickBrown1 PatrickBrown1 changed the title Pb/fix menu item dismiss popover fix(Select2, MenuItem2) fix Select2 ignoring MenuItem2 shouldDismissPopover Apr 15, 2023
@adidahiya
Copy link
Contributor

fix default to close

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor

removed unneeded comment

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

nice one, thanks @PatrickBrown1!

@adidahiya adidahiya changed the title fix(Select2, MenuItem2) fix Select2 ignoring MenuItem2 shouldDismissPopover [select] fix(Select2): check shouldDismissPopover on menu items Apr 17, 2023
@adidahiya adidahiya merged commit 6dab069 into develop Apr 17, 2023
@adidahiya adidahiya deleted the pb/fix-menu-item-dismiss-popover branch April 17, 2023 21:34
@adidahiya
Copy link
Contributor

It looks like this has broken the default dismissal behavior of TimezoneSelect items:

2023-04-19 15 15 04

it should be:

2023-04-19 15 15 21

@adidahiya
Copy link
Contributor

strike that -- this PR didn't break it, it actually surfaced current buggy behavior. we're specifying shouldDismissPopover={false} on those items, when we really shouldn't be. I'll push a bugfix PR.

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.

Select - handleClick in a MenuItem ignores "shouldDismissPopover"
2 participants