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

fix(#1511) fix click outside popover #2344

Draft
wants to merge 1 commit into
base: alpha
Choose a base branch
from

Conversation

BumbleB2na
Copy link
Collaborator

@BumbleB2na BumbleB2na commented Jan 9, 2025

Before (the change)

Dropdown uses Popover to show the list of options. Popovers had an invisible background behind them that user could click to close the popover. This prevented a user from clicking on anything behind the invisible background, like a hyperlink for example, until they clicked once to close the popover. This does not happen with a regular select. This problem affected Popover, Dropdown, AppHeader, AppHeaderMenu, and DatePicker.

After (the change)

Popover no longer has an invisible background and the component now handles a focusout event to decide if popover should be closed. Now it is possible to click outside of a popover to immediately interact with other elements, which is how a native select behaves:

1211-fix-click-outside-popover.mp4

Technical info

Popover no longer has an invisible background and the component now handles a focusout event for the same behavior. If the popover container or any element within that container loses focus, then focusout event bubbles up. It bubbles up from slotted children. When handling focusout it checks if the now active element is contained within the popover container by recursively looking down within nested slots. Whenever a popover open it also checks in the default slot used for content to see if any immediate children do not have tabindex set and if not then it adds tabindex="-1" which allows the focusout event to bubble up from these non-focusable elements. Another key change that was made is to always render popover contents to the DOM that are hidden when popover is closed.. previously it would not render those contents to the DOM, which was fine most of the time but causes edge case problems. This change makes the page more accessible, making it an additional fix. Take dropdown for example: the contents of a dropdown's options are now always part of the DOM just like how a native select options are always part of the DOM, so I think this is a positive change, and fwiw this change was needed to make Datepicker work well. Previously in DatePicker when you had one of its Month or Year dropdowns expanded, then if you click outside the calendar to collapse the entire Calendar popover, that dropdown will remain expanded next time the Calendar popover opens. This no longer happens.

Make sure that you've checked the boxes below before you submit the PR

  • I have read and followed the setup steps
  • I have created necessary unit tests
  • I have tested the functionality in both React and Angular.
    • tested in React
    • tested in Angular

Steps needed to test

Try this out with Dropdown, DatePicker, or the dropdown navigation within the App Header Menu. While any dropdown, datepicker, or sub-navigation is expanded, try clicking anywhere inside of the expanded content and it should not collapse. Then try clicking outside of the expanded content and it should collapse. This is all demonstrated in the video.

@BumbleB2na BumbleB2na force-pushed the 1511-fix-click-outside-popover branch 4 times, most recently from 7bc78aa to afd8b98 Compare January 11, 2025 17:33
@BumbleB2na BumbleB2na changed the title #1511 fix click outside popover fix(#1511) fix click outside popover Jan 11, 2025
@BumbleB2na BumbleB2na linked an issue Jan 11, 2025 that may be closed by this pull request
@BumbleB2na BumbleB2na force-pushed the 1511-fix-click-outside-popover branch 2 times, most recently from f0ef2dd to 580a83c Compare January 11, 2025 18:40
@ArakTaiRoth
Copy link
Collaborator

Very weird, and extremely specific issue with the Datepicker. This can only be replicated in Chromium browsers, although I haven't tested Safari yet. These are the Replication Steps, following them exactly:

  1. Setup a Datepicker, anything will do, angular or react, doesn't matter
  2. Click the Calendar icon to open the Calendar, specifically click the icon, not slightly off the icon
  3. Try to click away, no matter where you click you can't close the Calendar
  4. You can still click links to navigate away, and you can still use the calendar as normal, and once you do, it will close just fine

@BumbleB2na BumbleB2na force-pushed the 1511-fix-click-outside-popover branch from c5d6582 to 6c76fd9 Compare January 31, 2025 23:53
@BumbleB2na
Copy link
Collaborator Author

Very weird, and extremely specific issue with the Datepicker. This can only be replicated in Chromium browsers, although I haven't tested Safari yet. These are the Replication Steps, following them exactly:

  1. Setup a Datepicker, anything will do, angular or react, doesn't matter
  2. Click the Calendar icon to open the Calendar, specifically click the icon, not slightly off the icon
  3. Try to click away, no matter where you click you can't close the Calendar
  4. You can still click links to navigate away, and you can still use the calendar as normal, and once you do, it will close just fine

Thanks this revealed a problem with popover target that's been fixed

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.

Dropdown: 2 clicks to remove focus
2 participants