Skip to content

Commit c21659d

Browse files
committed
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
1 parent c92fb1c commit c21659d

1 file changed

Lines changed: 17 additions & 12 deletions

File tree

src/dropdown.ts

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -197,22 +197,24 @@ export class Dropdown extends Component<DropdownOptions> implements Openable {
197197
}
198198

199199
_setupTemporaryEventHandlers() {
200-
// Use capture phase event handler to prevent click
201-
document.body.addEventListener('click', this._handleDocumentClick, true);
200+
document.body.addEventListener('click', this._handleDocumentClick);
202201
document.body.addEventListener('touchmove', this._handleDocumentTouchmove);
203202
this.dropdownEl.addEventListener('keydown', this._handleDropdownKeydown);
204203
}
205204

206205
_removeTemporaryEventHandlers() {
207-
// Use capture phase event handler to prevent click
208-
document.body.removeEventListener('click', this._handleDocumentClick, true);
206+
document.body.removeEventListener('click', this._handleDocumentClick);
209207
document.body.removeEventListener('touchmove', this._handleDocumentTouchmove);
210208
this.dropdownEl.removeEventListener('keydown', this._handleDropdownKeydown);
211209
}
212210

213211
_handleClick = (e: MouseEvent) => {
214212
e.preventDefault();
215-
this.open();
213+
if (this.isOpen) {
214+
this.close();
215+
} else {
216+
this.open();
217+
}
216218
}
217219

218220
_handleMouseEnter = () => {
@@ -245,17 +247,18 @@ export class Dropdown extends Component<DropdownOptions> implements Openable {
245247
!this.isTouchMoving
246248
) {
247249
// isTouchMoving to check if scrolling on mobile.
248-
//setTimeout(() => {
249250
this.close();
250-
//}, 0);
251251
}
252252
else if (
253-
target.closest('.dropdown-trigger') ||
254253
!target.closest('.dropdown-content')
255254
) {
256-
//setTimeout(() => {
257-
this.close();
258-
//}, 0);
255+
// Do this one frame later so that if the element clicked also triggers _handleClick
256+
// For example, if a label for a select was clicked, that we don't close/open the dropdown
257+
setTimeout(() => {
258+
if (this.isOpen) {
259+
this.close();
260+
}
261+
}, 0);
259262
}
260263
this.isTouchMoving = false;
261264
}
@@ -592,7 +595,9 @@ export class Dropdown extends Component<DropdownOptions> implements Openable {
592595
this.dropdownEl.style.display = 'block';
593596
this._placeDropdown();
594597
this._animateIn();
595-
this._setupTemporaryEventHandlers();
598+
// Do this one frame later so that we don't bind an event handler that's immediately
599+
// called when the event bubbles up to the document and closes the dropdown
600+
setTimeout(() => this._setupTemporaryEventHandlers(), 0);
596601
}
597602

598603
/**

0 commit comments

Comments
 (0)