From c21659de11b71ef9f07f9e1f1399b1e0ffc437c2 Mon Sep 17 00:00:00 2001 From: Warren Spencer Date: Wed, 6 Mar 2024 07:03:32 -0800 Subject: [PATCH] fix: issue-456 dropdown open/close bug fix bug causing menu to toggle - Fix a bug where multiple clicks on a dropdown trigger would cause the dropdown to open/close on the 2nd+ clicks - Behaviour would also occur for the select component which leverages the dropdown component - Special consideration was given to the scenario where a select component has a label; where upon clicking the label the dropdown's _handleClick would be called, but the click event would not contain the dropdown-trigger (which is the select). Which would previously call both the _handleClick and _handleDocumentClick functions causing the same open/close behaviour --- src/dropdown.ts | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/dropdown.ts b/src/dropdown.ts index 7d9134dd9e..a0b472525e 100644 --- a/src/dropdown.ts +++ b/src/dropdown.ts @@ -197,22 +197,24 @@ export class Dropdown extends Component implements Openable { } _setupTemporaryEventHandlers() { - // Use capture phase event handler to prevent click - document.body.addEventListener('click', this._handleDocumentClick, true); + document.body.addEventListener('click', this._handleDocumentClick); document.body.addEventListener('touchmove', this._handleDocumentTouchmove); this.dropdownEl.addEventListener('keydown', this._handleDropdownKeydown); } _removeTemporaryEventHandlers() { - // Use capture phase event handler to prevent click - document.body.removeEventListener('click', this._handleDocumentClick, true); + document.body.removeEventListener('click', this._handleDocumentClick); document.body.removeEventListener('touchmove', this._handleDocumentTouchmove); this.dropdownEl.removeEventListener('keydown', this._handleDropdownKeydown); } _handleClick = (e: MouseEvent) => { e.preventDefault(); - this.open(); + if (this.isOpen) { + this.close(); + } else { + this.open(); + } } _handleMouseEnter = () => { @@ -245,17 +247,18 @@ export class Dropdown extends Component implements Openable { !this.isTouchMoving ) { // isTouchMoving to check if scrolling on mobile. - //setTimeout(() => { this.close(); - //}, 0); } else if ( - target.closest('.dropdown-trigger') || !target.closest('.dropdown-content') ) { - //setTimeout(() => { - this.close(); - //}, 0); + // Do this one frame later so that if the element clicked also triggers _handleClick + // For example, if a label for a select was clicked, that we don't close/open the dropdown + setTimeout(() => { + if (this.isOpen) { + this.close(); + } + }, 0); } this.isTouchMoving = false; } @@ -592,7 +595,9 @@ export class Dropdown extends Component implements Openable { this.dropdownEl.style.display = 'block'; this._placeDropdown(); this._animateIn(); - this._setupTemporaryEventHandlers(); + // Do this one frame later so that we don't bind an event handler that's immediately + // called when the event bubbles up to the document and closes the dropdown + setTimeout(() => this._setupTemporaryEventHandlers(), 0); } /**