From aef38f1679c3e7734487f22eb6b002816451b0ed Mon Sep 17 00:00:00 2001 From: Matiss Janis Aboltins Date: Mon, 31 Mar 2025 18:15:17 +0100 Subject: [PATCH] :bug: fix menu not closing when menu item is clicked (#4716) --- packages/component-library/src/Menu.tsx | 19 +- .../e2e/page-models/mobile-budget-page.ts | 30 ++- .../page-models/mobile-category-menu-modal.ts | 6 +- .../src/components/accounts/Header.tsx | 60 +++--- .../components/modals/CreateAccountModal.tsx | 88 ++++---- .../src/components/reports/Overview.tsx | 202 +++++++++--------- upcoming-release-notes/4716.md | 6 + 7 files changed, 232 insertions(+), 179 deletions(-) create mode 100644 upcoming-release-notes/4716.md diff --git a/packages/component-library/src/Menu.tsx b/packages/component-library/src/Menu.tsx index 90b341b117..1b95e1dfeb 100644 --- a/packages/component-library/src/Menu.tsx +++ b/packages/component-library/src/Menu.tsx @@ -3,11 +3,13 @@ import { useEffect, useRef, useState, + type ComponentProps, type ComponentType, type SVGProps, type CSSProperties, } from 'react'; +import { Button } from './Button'; import { Text } from './Text'; import { theme } from './theme'; import { Toggle } from './Toggle'; @@ -61,6 +63,7 @@ type MenuProps = { style?: CSSProperties; className?: string; getItemStyle?: (item: MenuItemObject) => CSSProperties; + slot?: ComponentProps['slot']; }; export function Menu({ @@ -71,6 +74,7 @@ export function Menu({ style, className, getItemStyle, + slot, }: MenuProps) { const elRef = useRef(null); const items = allItems.filter(x => x); @@ -161,9 +165,10 @@ export function Menu({ const Icon = item.icon; return ( - ({ }), ...(!isLabel(item) && getItemStyle?.(item)), }} - onPointerEnter={() => setHoveredIndex(idx)} - onPointerLeave={() => setHoveredIndex(null)} - onPointerUp={e => { - e.stopPropagation(); - + onHoverStart={() => setHoveredIndex(idx)} + onHoverEnd={() => setHoveredIndex(null)} + onPress={() => { if ( !item.disabled && item.toggle === undefined && @@ -232,7 +235,7 @@ export function Menu({ )} {item.key && } - + ); })} {footer} diff --git a/packages/desktop-client/e2e/page-models/mobile-budget-page.ts b/packages/desktop-client/e2e/page-models/mobile-budget-page.ts index daa7804b27..d696f35798 100644 --- a/packages/desktop-client/e2e/page-models/mobile-budget-page.ts +++ b/packages/desktop-client/e2e/page-models/mobile-budget-page.ts @@ -180,7 +180,11 @@ export class MobileBudgetPage { const categoryButton = await this.#getButtonForCategory(categoryName); await categoryButton.click(); - return new CategoryMenuModal(this.page.getByRole('dialog')); + return new CategoryMenuModal( + this.page.getByRole('dialog', { + name: 'Modal dialog', + }), + ); } async #getButtonForCell( @@ -222,7 +226,11 @@ export class MobileBudgetPage { const budgetedButton = await this.getButtonForBudgeted(categoryName); await budgetedButton.click(); - return new BudgetMenuModal(this.page.getByRole('dialog')); + return new BudgetMenuModal( + this.page.getByRole('dialog', { + name: 'Modal dialog', + }), + ); } async openSpentPage(categoryName: string) { @@ -239,7 +247,11 @@ export class MobileBudgetPage { if (await balanceButton.isVisible()) { await balanceButton.click(); - return new BalanceMenuModal(this.page.getByRole('dialog')); + return new BalanceMenuModal( + this.page.getByRole('dialog', { + name: 'Modal dialog', + }), + ); } else { throw new Error( `Balance button for category ${categoryName} not found or not visible.`, @@ -324,7 +336,11 @@ export class MobileBudgetPage { } await budgetSummaryButton.click(); - return new EnvelopeBudgetSummaryModal(this.page.getByRole('dialog')); + return new EnvelopeBudgetSummaryModal( + this.page.getByRole('dialog', { + name: 'Modal dialog', + }), + ); } async #getButtonForTrackingBudgetSummary({ @@ -358,6 +374,10 @@ export class MobileBudgetPage { } await budgetSummaryButton.click(); - return new TrackingBudgetSummaryModal(this.page.getByRole('dialog')); + return new TrackingBudgetSummaryModal( + this.page.getByRole('dialog', { + name: 'Modal dialog', + }), + ); } } diff --git a/packages/desktop-client/e2e/page-models/mobile-category-menu-modal.ts b/packages/desktop-client/e2e/page-models/mobile-category-menu-modal.ts index 53319613e5..01f2d2358b 100644 --- a/packages/desktop-client/e2e/page-models/mobile-category-menu-modal.ts +++ b/packages/desktop-client/e2e/page-models/mobile-category-menu-modal.ts @@ -25,6 +25,10 @@ export class CategoryMenuModal { async editNotes() { await this.editNotesButton.click(); - return new EditNotesModal(this.page.getByRole('dialog')); + return new EditNotesModal( + this.page.getByRole('dialog', { + name: 'Modal dialog', + }), + ); } } diff --git a/packages/desktop-client/src/components/accounts/Header.tsx b/packages/desktop-client/src/components/accounts/Header.tsx index d08a9f3a45..bf2a75e742 100644 --- a/packages/desktop-client/src/components/accounts/Header.tsx +++ b/packages/desktop-client/src/components/accounts/Header.tsx @@ -5,7 +5,7 @@ import React, { type ReactNode, type ComponentProps, } from 'react'; -import { DialogTrigger } from 'react-aria-components'; +import { Dialog, DialogTrigger } from 'react-aria-components'; import { useHotkeys } from 'react-hotkeys-hook'; import { Trans, useTranslation } from 'react-i18next'; @@ -463,18 +463,20 @@ export function AccountHeader({ - + + + @@ -490,20 +492,23 @@ export function AccountHeader({ - + + + @@ -722,6 +727,7 @@ function AccountMenu({ return ( { onMenuSelect(item); }} diff --git a/packages/desktop-client/src/components/modals/CreateAccountModal.tsx b/packages/desktop-client/src/components/modals/CreateAccountModal.tsx index 4d07b96232..3ddb6e456e 100644 --- a/packages/desktop-client/src/components/modals/CreateAccountModal.tsx +++ b/packages/desktop-client/src/components/modals/CreateAccountModal.tsx @@ -1,5 +1,5 @@ import React, { useEffect, useState } from 'react'; -import { DialogTrigger } from 'react-aria-components'; +import { Dialog, DialogTrigger } from 'react-aria-components'; import { Trans, useTranslation } from 'react-i18next'; import { Button, ButtonWithLoading } from '@actual-app/components/button'; @@ -416,19 +416,21 @@ export function CreateAccountModal({ - { - if (item === 'reconfigure') { - onGoCardlessReset(); - } - }} - items={[ - { - name: 'reconfigure', - text: t('Reset GoCardless credentials'), - }, - ]} - /> + + { + if (item === 'reconfigure') { + onGoCardlessReset(); + } + }} + items={[ + { + name: 'reconfigure', + text: t('Reset GoCardless credentials'), + }, + ]} + /> + )} @@ -479,19 +481,21 @@ export function CreateAccountModal({ /> - { - if (item === 'reconfigure') { - onSimpleFinReset(); - } - }} - items={[ - { - name: 'reconfigure', - text: t('Reset SimpleFIN credentials'), - }, - ]} - /> + + { + if (item === 'reconfigure') { + onSimpleFinReset(); + } + }} + items={[ + { + name: 'reconfigure', + text: t('Reset SimpleFIN credentials'), + }, + ]} + /> + )} @@ -543,19 +547,23 @@ export function CreateAccountModal({ - { - if (item === 'reconfigure') { - onPluggyAiReset(); - } - }} - items={[ - { - name: 'reconfigure', - text: t('Reset Pluggy.ai credentials'), - }, - ]} - /> + + { + if (item === 'reconfigure') { + onPluggyAiReset(); + } + }} + items={[ + { + name: 'reconfigure', + text: t( + 'Reset Pluggy.ai credentials', + ), + }, + ]} + /> + )} diff --git a/packages/desktop-client/src/components/reports/Overview.tsx b/packages/desktop-client/src/components/reports/Overview.tsx index f749b8b397..f751da8171 100644 --- a/packages/desktop-client/src/components/reports/Overview.tsx +++ b/packages/desktop-client/src/components/reports/Overview.tsx @@ -1,5 +1,5 @@ import React, { useMemo, useState } from 'react'; -import { DialogTrigger } from 'react-aria-components'; +import { Dialog, DialogTrigger } from 'react-aria-components'; import { Responsive, WidthProvider, type Layout } from 'react-grid-layout'; import { useHotkeys } from 'react-hotkeys-hook'; import { Trans, useTranslation } from 'react-i18next'; @@ -339,73 +339,76 @@ export function Overview() { - { - if (item === 'custom-report') { - navigate('/reports/custom'); - return; - } + + { + if (item === 'custom-report') { + navigate('/reports/custom'); + return; + } - function isExistingCustomReport( - name: string, - ): name is `custom-report-${string}` { - return name.startsWith('custom-report-'); - } - if (isExistingCustomReport(item)) { - const [, reportId] = item.split('custom-report-'); - onAddWidget('custom-report', { - id: reportId, - }); - return; - } + function isExistingCustomReport( + name: string, + ): name is `custom-report-${string}` { + return name.startsWith('custom-report-'); + } + if (isExistingCustomReport(item)) { + const [, reportId] = item.split('custom-report-'); + onAddWidget('custom-report', { + id: reportId, + }); + return; + } - if (item === 'markdown-card') { - onAddWidget(item, { - content: `### ${t('Text Widget')}\n\n${t('Edit this widget to change the **markdown** content.')}`, - }); - return; - } + if (item === 'markdown-card') { + onAddWidget(item, { + content: `### ${t('Text Widget')}\n\n${t('Edit this widget to change the **markdown** content.')}`, + }); + return; + } - onAddWidget(item); - }} - items={[ - { - name: 'cash-flow-card' as const, - text: t('Cash flow graph'), - }, - { - name: 'net-worth-card' as const, - text: t('Net worth graph'), - }, - { - name: 'spending-card' as const, - text: t('Spending analysis'), - }, - { - name: 'markdown-card' as const, - text: t('Text widget'), - }, - { - name: 'summary-card' as const, - text: t('Summary card'), - }, - { - name: 'calendar-card' as const, - text: t('Calendar card'), - }, - { - name: 'custom-report' as const, - text: t('New custom report'), - }, - ...(customReports.length - ? ([Menu.line] satisfies Array) - : []), - ...customReports.map(report => ({ - name: `custom-report-${report.id}` as const, - text: report.name, - })), - ]} - /> + onAddWidget(item); + }} + items={[ + { + name: 'cash-flow-card' as const, + text: t('Cash flow graph'), + }, + { + name: 'net-worth-card' as const, + text: t('Net worth graph'), + }, + { + name: 'spending-card' as const, + text: t('Spending analysis'), + }, + { + name: 'markdown-card' as const, + text: t('Text widget'), + }, + { + name: 'summary-card' as const, + text: t('Summary card'), + }, + { + name: 'calendar-card' as const, + text: t('Calendar card'), + }, + { + name: 'custom-report' as const, + text: t('New custom report'), + }, + ...(customReports.length + ? ([Menu.line] satisfies Array) + : []), + ...customReports.map(report => ({ + name: `custom-report-${report.id}` as const, + text: report.name, + })), + ]} + /> + @@ -434,39 +437,42 @@ export function Overview() { /> - { - switch (item) { - case 'reset': - onResetDashboard(); - break; - case 'export': - onExport(); - break; - case 'import': - onImport(); - break; - } - }} - items={[ - { - name: 'reset', - text: t('Reset to default'), - disabled: isImporting, - }, - Menu.line, - { - name: 'import', - text: t('Import'), - disabled: isImporting, - }, - { - name: 'export', - text: t('Export'), - disabled: isImporting, - }, - ]} - /> + + { + switch (item) { + case 'reset': + onResetDashboard(); + break; + case 'export': + onExport(); + break; + case 'import': + onImport(); + break; + } + }} + items={[ + { + name: 'reset', + text: t('Reset to default'), + disabled: isImporting, + }, + Menu.line, + { + name: 'import', + text: t('Import'), + disabled: isImporting, + }, + { + name: 'export', + text: t('Export'), + disabled: isImporting, + }, + ]} + /> + diff --git a/upcoming-release-notes/4716.md b/upcoming-release-notes/4716.md new file mode 100644 index 0000000000..4ef6452c69 --- /dev/null +++ b/upcoming-release-notes/4716.md @@ -0,0 +1,6 @@ +--- +category: Bugfix +authors: [MatissJanis] +--- + +Fix menu not closing when menu item is clicked