fix(popovercontainer): prevent immediate re-toggle #2695
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This small timeout prevents the popovercontainer from opening and closing with the same event chain. This happens for example when the popover is open and the trigger button is clicked again. It's because first, an outside click is registered the popover closes. Right after that, the click from the toggle button fires and opens the popover again.
🦋 Changeset detectedLatest commit: 610846a The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This removes some complexity from popover and tooltips since the popovercontainer now handles instant re-opens and also some observer code can be abstracted into a factory function.
|
Preview environment ready: https://preview-2695--swisspost-design-system-next.netlify.app |
imagoiq
approved these changes
Feb 20, 2024
| @@ -9,6 +9,7 @@ module.exports = defineConfig({ | |||
| viewportHeight: 576, | |||
| }, | |||
| includeShadowDom: true, | |||
| chromeWebSecurity: false, | |||
There was a problem hiding this comment.
Can you please explain why it is necessary. Here is enough.
There was a problem hiding this comment.
For some reason I had CORS issues when testing locally. I might be able to remove it again, will retest.
Comment on lines
15
to
18
| if (!target || !('getAttribute' in target)) return; | ||
| const popoverTarget = target.getAttribute(popoverTargetAttribute); | ||
| if (!popoverTarget || popoverTarget === '') return; | ||
| if ('key' in e && e.key !== 'Enter') return; |
There was a problem hiding this comment.
It's not easy to read. Maybe you can extract those conditions into variables and have a single if condition?
| @@ -47,6 +47,7 @@ export class PostPopovercontainer { | |||
| private arrowRef: HTMLElement; | |||
| private eventTarget: Element; | |||
| private clearAutoUpdate: () => void; | |||
| private toggleTimeout: number; | |||
There was a problem hiding this comment.
At first read, I thought that this variable contain an actual time value, but it's rather the id of the setTimeout. Also it can contain null on line 131
|
gfellerph
pushed a commit
that referenced
this pull request
Feb 29, 2024
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @swisspost/[email protected] ### Patch Changes - Improved display of tooltip arrow in high contrast mode. (by [@imagoiq](https://github.com/imagoiq) with [#2697](#2697)) - Fixes an issue with the post-popover component that stopped working once trigger buttons were removed from the page or new trigger buttons were added to the page asynchronously. (by [@gfellerph](https://github.com/gfellerph) with [#2695](#2695)) - Updated dependencies: - @swisspost/[email protected] ## @swisspost/[email protected] ### Patch Changes - Updated dependencies: - @swisspost/[email protected] ## @swisspost/[email protected] ### Patch Changes - Reinstated original navigation role for post-main-navigation. (by [@imagoiq](https://github.com/imagoiq) with [#2709](#2709)) - Updated dependencies: - @swisspost/[email protected] ## @swisspost/[email protected] ### Patch Changes - Updated dependencies: - @swisspost/[email protected] ## @swisspost/[email protected] ### Patch Changes - Fixed missing migration files. (by [@alizedebray](https://github.com/alizedebray) with [#2712](#2712)) ## @swisspost/[email protected] ### Patch Changes - Updated the color of success alerts from black to white for a better contrast. (by [@alizedebray](https://github.com/alizedebray) with [#2740](#2740)) - Fixed color-contrast on blockquote footer with dark background. (by [@imagoiq](https://github.com/imagoiq) with [#2714](#2714)) - Fixed close icon button which is not visible on hover with light theme and high contrast mode enabled. (by [@imagoiq](https://github.com/imagoiq) with [#2705](#2705)) - Improved display of badge and switch checked state with high contrast mode. (by [@imagoiq](https://github.com/imagoiq) with [#2706](#2706)) - Reduced the `xxl` breakpoint size form 1441px to 1440ppx. (by [@alizedebray](https://github.com/alizedebray) with [#2741](#2741)) - Fixed color-contrast issue on valid form-feedback. (by [@imagoiq](https://github.com/imagoiq) with [#2717](#2717)) - Removes unwanted margin from heading within the notification overlay component. (by [@b1aserlu](https://github.com/b1aserlu) with [#2407](#2407)) ## @swisspost/[email protected] ### Minor Changes - Added a new documentation page about metadata. It has links to documentations about most common metadata of a webpage as well as an example of an HTML header. (by [@b1aserlu](https://github.com/b1aserlu) with [#2511](#2511)) - Added a Button to the toolbar of Storybook to visit older versions of the documentation. (by [@b1aserlu](https://github.com/b1aserlu) with [#2635](#2635)) - Added a documentation page for the Subnavigation Component. (by [@davidritter-dotcom](https://github.com/davidritter-dotcom) with [#2574](#2574)) ### Patch Changes - Deprecated `rg` breakpoint. (by [@oliverschuerch](https://github.com/oliverschuerch) with [#2238](#2238)) - Removed the info banner telling everyone that the storybook docs are in beta. They've grown up now. (by [@gfellerph](https://github.com/gfellerph) with [#2739](#2739)) - Added example for intranet-header component with optionDropdownContent. (by [@imagoiq](https://github.com/imagoiq) with [#2719](#2719)) - Added color value after color title to help for comparison. (by [@imagoiq](https://github.com/imagoiq) with [#2730](#2730)) - Updated dependencies: - @swisspost/[email protected] - @swisspost/[email protected] - @swisspost/[email protected] - @swisspost/[email protected] - @swisspost/[email protected] - @swisspost/[email protected] ## @swisspost/[email protected] ### Patch Changes - Updated dependencies: - @swisspost/[email protected] - @swisspost/[email protected] ## @swisspost/[email protected] ### Patch Changes - Updated dependencies: - @swisspost/[email protected] ## @swisspost/[email protected] ### Patch Changes - Updated dependencies: - @swisspost/[email protected] - @swisspost/[email protected] ## @swisspost/[email protected] ### Patch Changes - Updated dependencies: - @swisspost/[email protected] ## @swisspost/[email protected] ### Patch Changes - Updated dependencies: - @swisspost/[email protected] - @swisspost/[email protected] - @swisspost/[email protected] Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This small timeout prevents the popovercontainer from opening and closing with the same event chain. This happens for example when the popover is open and the trigger button is clicked again. It's because first, an outside click is registered the popover closes. Right after that, the click from the toggle button fires and opens the popover again.