Skip to content

Fix network dropdown button#10312

Merged
Gudahtt merged 1 commit intodevelopfrom
fix-network-dropdown-close
Jan 30, 2021
Merged

Fix network dropdown button#10312
Gudahtt merged 1 commit intodevelopfrom
fix-network-dropdown-close

Conversation

@Gudahtt
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt commented Jan 28, 2021

This fixes a bug where the network menu would remain present after a second click on the network menu button. The bug was caused by the click being handled twice, by two separate handlers. First it was caught by the external click handler of the dropdown menu, which closed the menu. Second, it was caught by the network button itself, which re-opened the menu. This all happens quickly enough that to the user it appears to stay open.

The external click handler of the menu now only fires if the menu is open. Additionally, any click that is caught by the network menu is stopped from propagating further, so that it can't trigger additional click handlers.

Manual testing steps:

  • Click the network menu button
  • Click it again. See that it closes the network menu.

@Gudahtt
Copy link
Copy Markdown
Member Author

Gudahtt commented Jan 28, 2021

This depends upon #10311

brad-decker
brad-decker previously approved these changes Jan 29, 2021
Copy link
Copy Markdown
Contributor

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

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

LGTM, i'll reapprove after the other one merges.

Base automatically changed from fix-menu-droppo-external-click-handler to develop January 29, 2021 20:34
@Gudahtt Gudahtt dismissed brad-decker’s stale review January 29, 2021 20:34

The base branch was changed.

This fixes a bug where the network menu would remain present after a
second click on the network menu button. The bug was caused by the
click being handled _twice_, by two separate handlers. First it was
caught by the external click handler of the dropdown menu, which closed
the menu. Second, it was caught by the network button itself, which re-
opened the menu. This all happens quickly enough that to the user it
appears to stay open.

The external click handler of the menu now only fires if the menu is
open. Additionally, any click that is caught by the network menu is
stopped from propagating further, so that it can't trigger additional
click handlers.
@Gudahtt Gudahtt force-pushed the fix-network-dropdown-close branch from 99864a6 to 7a3ac1b Compare January 29, 2021 21:24
@Gudahtt Gudahtt marked this pull request as ready for review January 29, 2021 21:24
@Gudahtt Gudahtt requested a review from a team as a code owner January 29, 2021 21:24
@Gudahtt Gudahtt requested a review from darkwing January 29, 2021 21:24
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [7a3ac1b]
Page Load Metrics (748 ± 97 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaintNaNNaNNaNNaNNaN
domContentLoaded365100874419895
load366104474820297
domInteractive365100774419895

@Gudahtt Gudahtt merged commit f381f41 into develop Jan 30, 2021
@Gudahtt Gudahtt deleted the fix-network-dropdown-close branch January 30, 2021 15:42
@github-actions github-actions bot locked and limited conversation to collaborators Jan 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants