From d87a6f70683f02bce805854e76077c24ee1361f8 Mon Sep 17 00:00:00 2001 From: pengx17 Date: Tue, 26 Nov 2024 08:39:09 +0000 Subject: [PATCH] fix(core): multi sub menu layer handling (#8916) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix AF-1800, AF-1801
🎥 Video uploaded on Graphite:
--- .../component/src/ui/menu/mobile/context.ts | 53 ++++++++++++++++++- .../component/src/ui/menu/mobile/item.tsx | 12 +++-- .../component/src/ui/menu/mobile/root.tsx | 34 +++++++++--- .../component/src/ui/menu/mobile/sub.tsx | 26 +++++++-- .../src/components/doc-properties/table.tsx | 11 ++-- tests/affine-mobile/e2e/detail.spec.ts | 39 +++++++++++--- 6 files changed, 147 insertions(+), 28 deletions(-) diff --git a/packages/frontend/component/src/ui/menu/mobile/context.ts b/packages/frontend/component/src/ui/menu/mobile/context.ts index fde6245de7..d0377236aa 100644 --- a/packages/frontend/component/src/ui/menu/mobile/context.ts +++ b/packages/frontend/component/src/ui/menu/mobile/context.ts @@ -3,6 +3,8 @@ import { type Dispatch, type ReactNode, type SetStateAction, + useCallback, + useContext, } from 'react'; import type { MenuSubProps } from '../menu.types'; @@ -14,14 +16,61 @@ export type SubMenuContent = { */ title?: string; items: ReactNode; + options?: MenuSubProps['subOptions']; contentOptions?: MenuSubProps['subContentOptions']; + id: string; }; -export const MobileMenuContext = createContext<{ +export type MobileMenuContextValue = { subMenus: Array; setSubMenus: Dispatch>>; setOpen?: (v: boolean) => void; -}>({ +}; + +export const MobileMenuContext = createContext({ subMenus: [], setSubMenus: () => {}, }); + +export const useMobileSubMenuHelper = ( + contextValue?: MobileMenuContextValue +) => { + const _context = useContext(MobileMenuContext); + const { subMenus, setSubMenus } = contextValue ?? _context; + + const addSubMenu = useCallback( + (subMenu: SubMenuContent) => { + const id = subMenu.id; + // if the submenu already exists, do nothing + if (subMenus.some(sub => sub.id === id)) { + return; + } + subMenu.options?.onOpenChange?.(true); + setSubMenus(prev => { + return [...prev, subMenu]; + }); + }, + [setSubMenus, subMenus] + ); + + const removeSubMenu = useCallback( + (id: string) => { + setSubMenus(prev => { + const index = prev.findIndex(sub => sub.id === id); + prev[index]?.options?.onOpenChange?.(false); + return prev.filter(sub => sub.id !== id); + }); + }, + [setSubMenus] + ); + + const removeAllSubMenus = useCallback(() => { + setSubMenus([]); + }, [setSubMenus]); + + return { + addSubMenu, + removeSubMenu, + removeAllSubMenus, + }; +}; diff --git a/packages/frontend/component/src/ui/menu/mobile/item.tsx b/packages/frontend/component/src/ui/menu/mobile/item.tsx index f211f5172d..e7c3bd4ed4 100644 --- a/packages/frontend/component/src/ui/menu/mobile/item.tsx +++ b/packages/frontend/component/src/ui/menu/mobile/item.tsx @@ -10,7 +10,7 @@ const preventDefault = () => { }; export const MobileMenuItem = (props: MenuItemProps) => { - const { setOpen } = useContext(MobileMenuContext); + const { setOpen, subMenus, setSubMenus } = useContext(MobileMenuContext); const { className, children, otherProps } = useMenuItem(props); const { onSelect, onClick, divide, ...restProps } = otherProps; @@ -21,10 +21,16 @@ export const MobileMenuItem = (props: MenuItemProps) => { if (preventDefaultFlag) { preventDefaultFlag = false; } else { - setOpen?.(false); + if (subMenus.length > 1) { + // assume the user can only click the last menu + // (mimic the back button) + setSubMenus(subMenus.slice(0, -1)); + } else { + setOpen?.(false); + } } }, - [onClick, onSelect, setOpen] + [onClick, onSelect, setOpen, setSubMenus, subMenus] ); return ( diff --git a/packages/frontend/component/src/ui/menu/mobile/root.tsx b/packages/frontend/component/src/ui/menu/mobile/root.tsx index 13a6caac53..e1530ff84e 100644 --- a/packages/frontend/component/src/ui/menu/mobile/root.tsx +++ b/packages/frontend/component/src/ui/menu/mobile/root.tsx @@ -9,8 +9,11 @@ import { Button } from '../../button'; import { Modal } from '../../modal'; import { Scrollable } from '../../scrollbar'; import type { MenuProps } from '../menu.types'; -import type { SubMenuContent } from './context'; -import { MobileMenuContext } from './context'; +import { + MobileMenuContext, + type SubMenuContent, + useMobileSubMenuHelper, +} from './context'; import * as styles from './styles.css'; import { MobileMenuSubRaw } from './sub'; @@ -32,12 +35,23 @@ export const MobileMenu = ({ }: MenuProps) => { const [subMenus, setSubMenus] = useState([]); const [open, setOpen] = useState(false); + const mobileContextValue = { + subMenus, + setSubMenus, + setOpen, + }; + + const { removeSubMenu, removeAllSubMenus } = + useMobileSubMenuHelper(mobileContextValue); + const [sliderHeight, setSliderHeight] = useState(0); const [sliderElement, setSliderElement] = useState( null ); const { setOpen: pSetOpen } = useContext(MobileMenuContext); const finalOpen = rootOptions?.open ?? open; + + // always show the last submenu, if any const activeIndex = subMenus.length; // dynamic height for slider @@ -62,12 +76,12 @@ export const MobileMenu = ({ // a workaround to hack the onPointerDownOutside event onPointerDownOutside?.({} as any); onInteractOutside?.({} as any); - setSubMenus([]); + removeAllSubMenus(); } setOpen(open); rootOptions?.onOpenChange?.(open); }, - [onInteractOutside, onPointerDownOutside, rootOptions] + [onInteractOutside, onPointerDownOutside, removeAllSubMenus, rootOptions] ); const onItemClick = useCallback( @@ -93,7 +107,11 @@ export const MobileMenu = ({ * ``` */ if (pSetOpen) { - return {children}; + return ( + + {children} + + ); } return ( @@ -126,7 +144,7 @@ export const MobileMenu = ({ {subMenus.map((sub, index) => (
@@ -135,7 +153,9 @@ export const MobileMenu = ({ variant="plain" className={styles.backButton} prefix={} - onClick={() => setSubMenus(prev => prev.slice(0, index))} + onClick={() => { + removeSubMenu(sub.id); + }} prefixStyle={{ width: 24, height: 24 }} > {sub.title || t['com.affine.backButton']()} diff --git a/packages/frontend/component/src/ui/menu/mobile/sub.tsx b/packages/frontend/component/src/ui/menu/mobile/sub.tsx index eea419ada0..30c1a26485 100644 --- a/packages/frontend/component/src/ui/menu/mobile/sub.tsx +++ b/packages/frontend/component/src/ui/menu/mobile/sub.tsx @@ -1,10 +1,10 @@ import { ArrowRightSmallPlusIcon } from '@blocksuite/icons/rc'; import { Slot } from '@radix-ui/react-slot'; -import { type MouseEvent, useCallback, useContext } from 'react'; +import { type MouseEvent, useCallback, useEffect, useId, useMemo } from 'react'; import type { MenuSubProps } from '../menu.types'; import { useMenuItem } from '../use-menu-item'; -import { MobileMenuContext } from './context'; +import { useMobileSubMenuHelper } from './context'; export const MobileMenuSub = ({ title, @@ -42,20 +42,36 @@ export const MobileMenuSubRaw = ({ onClick, children, items, + subOptions, subContentOptions: contentOptions = {}, }: MenuSubProps & { onClick?: (e: MouseEvent) => void; title?: string; }) => { - const { setSubMenus } = useContext(MobileMenuContext); + const id = useId(); + const { addSubMenu } = useMobileSubMenuHelper(); + + const subMenuContent = useMemo( + () => ({ items, contentOptions, options: subOptions, title, id }), + [items, contentOptions, subOptions, title, id] + ); + + const doAddSubMenu = useCallback(() => { + addSubMenu(subMenuContent); + }, [addSubMenu, subMenuContent]); const onItemClick = useCallback( (e: MouseEvent) => { onClick?.(e); - setSubMenus(prev => [...prev, { items, contentOptions, title }]); + doAddSubMenu(); }, - [contentOptions, items, onClick, setSubMenus, title] + [doAddSubMenu, onClick] ); + useEffect(() => { + if (subOptions?.open) { + doAddSubMenu(); + } + }, [doAddSubMenu, subOptions]); return {children}; }; diff --git a/packages/frontend/core/src/components/doc-properties/table.tsx b/packages/frontend/core/src/components/doc-properties/table.tsx index 8404832da5..04c9b36121 100644 --- a/packages/frontend/core/src/components/doc-properties/table.tsx +++ b/packages/frontend/core/src/components/doc-properties/table.tsx @@ -250,7 +250,7 @@ const DocWorkspacePropertiesTableBody = forwardRef< const workbenchService = useService(WorkbenchService); const viewService = useServiceOptional(ViewService); const properties = useLiveData(docsService.propertyList.sortedProperties$); - const [propertyCollapsed, setPropertyCollapsed] = useState(true); + const [addMoreCollapsed, setAddMoreCollapsed] = useState(true); const [newPropertyId, setNewPropertyId] = useState(null); @@ -262,6 +262,10 @@ const DocWorkspacePropertiesTableBody = forwardRef< [onPropertyAdded] ); + const handleCollapseChange = useCallback(() => { + setNewPropertyId(null); + }, []); + return ( isCollapsed diff --git a/tests/affine-mobile/e2e/detail.spec.ts b/tests/affine-mobile/e2e/detail.spec.ts index 4357a0fc69..ba2faf545a 100644 --- a/tests/affine-mobile/e2e/detail.spec.ts +++ b/tests/affine-mobile/e2e/detail.spec.ts @@ -1,5 +1,13 @@ import { test } from '@affine-test/kit/mobile'; -import { expect } from '@playwright/test'; +import { expect, type Page } from '@playwright/test'; + +const openDocInfoModal = async (page: Page) => { + await page.click('[data-testid="detail-page-header-more-button"]'); + await expect(page.getByRole('dialog')).toBeVisible(); + + await page.getByRole('menuitem', { name: 'view info' }).click(); + await expect(page.getByTestId('mobile-menu-back-button')).toBeVisible(); +}; test.beforeEach(async ({ page }) => { const docsTab = page.locator('#app-tabs').getByRole('tab', { name: 'all' }); @@ -22,13 +30,28 @@ test('switch to page mode', async ({ page }) => { await expect(page.locator('.doc-title-container')).toBeVisible(); }); -test('doc info', async ({ page }) => { - await page.click('[data-testid="detail-page-header-more-button"]'); - await expect(page.getByRole('dialog')).toBeVisible(); - - await page.getByRole('menuitem', { name: 'view info' }).click(); - await expect(page.getByTestId('mobile-menu-back-button')).toBeVisible(); - +test('can show doc info', async ({ page }) => { + await openDocInfoModal(page); await expect(page.getByRole('dialog')).toContainText('Created'); await expect(page.getByRole('dialog')).toContainText('Updated'); }); + +test('can add text property', async ({ page }) => { + await openDocInfoModal(page); + + await expect( + page.getByRole('button', { name: 'Add property' }) + ).toBeVisible(); + + await page.getByRole('button', { name: 'Add property' }).click(); + await page.getByRole('menuitem', { name: 'Text' }).click(); + + await expect( + page.getByTestId('mobile-menu-back-button').last() + ).toBeVisible(); + await page.getByTestId('mobile-menu-back-button').last().click(); + + await expect(page.getByTestId('mobile-menu-back-button')).toContainText( + 'Write, Draw, Plan all at Once' + ); +});