Skip to content

Commit 1edd1ad

Browse files
n3pshjetpoluruDDDDDanica
authored
fix: Popover toggle (#35746)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Fixes the Popover so it can toggled by the same element that opened it [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/35746?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null ## **Related issues** Fixes: ## **Manual testing steps** 1. Home screen 2. Click menu button to open 3. Click same button to close ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: HJetpoluru <[email protected]> Co-authored-by: Danica Shen <[email protected]>
1 parent 2dad278 commit 1edd1ad

File tree

5 files changed

+49
-37
lines changed

5 files changed

+49
-37
lines changed

test/e2e/page-objects/pages/header-navbar.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,11 @@ class HeaderNavbar {
108108
}
109109
}
110110

111+
async mouseClickOnThreeDotMenu(): Promise<void> {
112+
console.log('Clicking three dot menu using mouse move');
113+
await this.driver.clickElementUsingMouseMove(this.threeDotMenuButton);
114+
}
115+
111116
async openPermissionsPage(): Promise<void> {
112117
console.log('Open permissions page in header navbar');
113118
await this.openThreeDotMenu();
@@ -140,12 +145,12 @@ class HeaderNavbar {
140145

141146
async clickNotificationsOptions(): Promise<void> {
142147
console.log('Click notifications options');
143-
await this.openThreeDotMenu();
148+
await this.mouseClickOnThreeDotMenu();
144149
await this.driver.clickElement(this.notificationsButton);
145150
}
146151

147152
async checkNotificationCountInMenuOption(count: number): Promise<void> {
148-
await this.openThreeDotMenu();
153+
await this.mouseClickOnThreeDotMenu();
149154
await this.driver.findElement({
150155
css: this.notificationCountOption,
151156
text: count.toString(),

test/e2e/snaps/test-snap-cronjob-duration.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ describe('Test Snap Cronjob Duration', function () {
4040
await headerNavbar.checkNotificationCountInMenuOption(1);
4141

4242
// This click will close the menu.
43-
await headerNavbar.openThreeDotMenu();
43+
await headerNavbar.mouseClickOnThreeDotMenu();
4444

4545
// Click the notification options and validate the message in the
4646
// notification list.

test/e2e/snaps/test-snap-notification.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ describe('Test Snap Notification', function () {
8585
await headerNavbar.checkNotificationCountInMenuOption(1);
8686

8787
// this click will close the menu
88-
await headerNavbar.openThreeDotMenu();
88+
await headerNavbar.mouseClickOnThreeDotMenu();
8989

9090
// click the notification options
9191
await headerNavbar.clickNotificationsOptions();

ui/components/component-library/popover/popover.tsx

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -86,29 +86,30 @@ export const Popover: PopoverComponent = React.forwardRef(
8686
width: matchWidth ? referenceElement?.clientWidth : 'auto',
8787
};
8888

89-
// Esc key press
90-
const handleEscKey = (event: KeyboardEvent) => {
91-
if (event.key === 'Escape') {
92-
// Close the popover when the "Esc" key is pressed
93-
if (onPressEscKey) {
94-
onPressEscKey();
89+
useEffect(() => {
90+
// Esc key press
91+
const handleEscKey = (event: KeyboardEvent) => {
92+
if (event.key === 'Escape') {
93+
// Close the popover when the "Esc" key is pressed
94+
if (onPressEscKey) {
95+
onPressEscKey();
96+
}
9597
}
96-
}
97-
};
98+
};
9899

99-
const handleClickOutside = (event: MouseEvent) => {
100-
if (
101-
isOpen &&
102-
popoverRef.current &&
103-
!popoverRef.current.contains(event.target as Node)
104-
) {
105-
if (onClickOutside) {
106-
onClickOutside();
100+
const handleClickOutside = (event: MouseEvent) => {
101+
if (
102+
isOpen &&
103+
popoverRef.current &&
104+
!popoverRef.current.contains(event.target as Node) &&
105+
!referenceElement?.contains(event.target as Node)
106+
) {
107+
if (onClickOutside) {
108+
onClickOutside();
109+
}
107110
}
108-
}
109-
};
111+
};
110112

111-
useEffect(() => {
112113
document.addEventListener('keydown', handleEscKey, { capture: true });
113114
if (isOpen) {
114115
document.addEventListener('click', handleClickOutside, {
@@ -122,7 +123,7 @@ export const Popover: PopoverComponent = React.forwardRef(
122123
document.removeEventListener('keydown', handleEscKey);
123124
document.removeEventListener('click', handleClickOutside);
124125
};
125-
}, [onPressEscKey, isOpen, onClickOutside]);
126+
}, [onPressEscKey, isOpen, onClickOutside, referenceElement]);
126127

127128
const PopoverContent = (
128129
<Box

ui/components/multichain/app-header/app-header-unlocked-content.tsx

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -139,15 +139,21 @@ export const AppHeaderUnlockedContent = ({
139139
origin &&
140140
origin !== browser.runtime.id;
141141

142-
const handleMainMenuOpened = () => {
143-
trackEvent({
144-
event: MetaMetricsEventName.NavMainMenuOpened,
145-
category: MetaMetricsEventCategory.Navigation,
146-
properties: {
147-
location: 'Home',
148-
},
142+
const handleMainMenuToggle = () => {
143+
setAccountOptionsMenuOpen((previous) => {
144+
const isMenuOpen = !previous;
145+
if (isMenuOpen) {
146+
trackEvent({
147+
event: MetaMetricsEventName.NavMainMenuOpened,
148+
category: MetaMetricsEventCategory.Navigation,
149+
properties: {
150+
location: 'Home',
151+
},
152+
});
153+
}
154+
155+
return isMenuOpen;
149156
});
150-
setAccountOptionsMenuOpen(true);
151157
};
152158

153159
const handleConnectionsRoute = () => {
@@ -284,25 +290,25 @@ export const AppHeaderUnlockedContent = ({
284290
style={{ position: 'relative' }}
285291
>
286292
{!accountOptionsMenuOpen && (
287-
<Box onClick={() => handleMainMenuOpened()}>
293+
<Box onClick={handleMainMenuToggle}>
288294
<NotificationsTagCounter noLabel />
289295
</Box>
290296
)}
291297
<ButtonIcon
292298
iconName={IconName.Menu}
293299
data-testid="account-options-menu-button"
294300
ariaLabel={t('accountOptions')}
295-
onClick={() => {
296-
handleMainMenuOpened();
297-
}}
301+
onClick={handleMainMenuToggle}
298302
size={ButtonIconSize.Lg}
299303
/>
300304
</Box>
301305
</Box>
302306
<GlobalMenu
303307
anchorElement={menuRef.current}
304308
isOpen={accountOptionsMenuOpen}
305-
closeMenu={() => setAccountOptionsMenuOpen(false)}
309+
closeMenu={() => {
310+
setAccountOptionsMenuOpen(false);
311+
}}
306312
/>
307313
<VisitSupportDataConsentModal
308314
isOpen={showSupportDataConsentModal}

0 commit comments

Comments
 (0)