fix(mobile): setting item popover cannot be closed (#8910)

fix AF-1798
toeverything · Nov 25, 2024 · f6eb84a · f6eb84a
1 parent b369ee0
commit f6eb84a
Show file tree

Hide file tree

Showing 8 changed files with 96 additions and 27 deletions.
diff --git a/packages/frontend/core/src/components/doc-properties/types/text.tsx b/packages/frontend/core/src/components/doc-properties/types/text.tsx
@@ -78,7 +78,7 @@ const MobileTextValue = ({
 }: PropertyValueProps) => {
   const [open, setOpen] = useState(false);
 
-  const [tempValue, setTempValue] = useState<string>(value);
+  const [tempValue, setTempValue] = useState<string>(value || '');
   const handleClick = useCallback((e: React.MouseEvent) => {
     e.stopPropagation();
     setOpen(true);
@@ -111,20 +111,21 @@ const MobileTextValue = ({
   }, [onChange, tempValue]);
   const t = useI18n();
   useEffect(() => {
-    setTempValue(value);
+    setTempValue(value || '');
   }, [value]);
 
   return (
-    <PropertyValue
-      className={styles.textPropertyValueContainer}
-      onClick={handleClick}
-      isEmpty={!value}
-    >
-      <div className={styles.mobileTextareaPlain} data-empty={!tempValue}>
-        {tempValue ||
-          t['com.affine.page-properties.property-value-placeholder']()}
-      </div>
-
+    <>
+      <PropertyValue
+        className={styles.textPropertyValueContainer}
+        onClick={handleClick}
+        isEmpty={!value}
+      >
+        <div className={styles.mobileTextareaPlain} data-empty={!tempValue}>
+          {tempValue ||
+            t['com.affine.page-properties.property-value-placeholder']()}
+        </div>
+      </PropertyValue>
       <ConfigModal
         open={open}
         onOpenChange={setOpen}
@@ -154,7 +155,7 @@ const MobileTextValue = ({
           </div>
         </div>
       </ConfigModal>
-    </PropertyValue>
+    </>
   );
 };
 

diff --git a/packages/frontend/core/src/components/mobile/config-modal/index.tsx b/packages/frontend/core/src/components/mobile/config-modal/index.tsx
@@ -44,9 +44,6 @@ export const ConfigModal = ({
       animation="slideBottom"
       withoutCloseButton
       contentOptions={{
-        onClick: e => {
-          e.stopPropagation();
-        },
         className:
           variant === 'page'
             ? styles.pageModalContent

diff --git a/packages/frontend/core/src/mobile/dialogs/setting/dropdown-select.tsx b/packages/frontend/core/src/mobile/dialogs/setting/dropdown-select.tsx
@@ -64,7 +64,11 @@ export const SettingDropdownSelect = <
       ))}
       {...menuOptions}
     >
-      <div className={clsx(styles.root, className)} {...attrs}>
+      <div
+        data-testid="dropdown-select-trigger"
+        className={clsx(styles.root, className)}
+        {...attrs}
+      >
         <span className={styles.label}>{selectedItem?.label ?? ''}</span>
 
         <ArrowDownSmallIcon className={styles.icon} />

diff --git a/packages/frontend/core/src/mobile/dialogs/setting/row.layout.tsx b/packages/frontend/core/src/mobile/dialogs/setting/row.layout.tsx
@@ -10,7 +10,10 @@ export const RowLayout = ({
   href,
 }: PropsWithChildren<{ label: ReactNode; href?: string }>) => {
   const content = (
-    <ConfigModal.Row className={styles.baseSettingItem}>
+    <ConfigModal.Row
+      data-testid="setting-row"
+      className={styles.baseSettingItem}
+    >
       <div className={styles.baseSettingItemName}>{label}</div>
       <div className={styles.baseSettingItemAction}>
         {children ||

diff --git a/packages/frontend/core/src/mobile/views/home-header/index.tsx b/packages/frontend/core/src/mobile/views/home-header/index.tsx
@@ -75,6 +75,7 @@ export const HomeHeader = () => {
           onClick={openSetting}
           size={28}
           icon={<SettingsIcon />}
+          data-testid="settings-button"
         />
       </SafeArea>
     </>

diff --git a/packages/frontend/core/src/modules/doc-info/views/database-properties/cells/rich-text.tsx b/packages/frontend/core/src/modules/doc-info/views/database-properties/cells/rich-text.tsx
@@ -97,7 +97,8 @@ const MobileRichTextCell = ({
   const [open, setOpen] = useState(false);
   const name = useLiveData(cell.property.name$);
   return (
-    <PropertyValue onClick={() => setOpen(true)}>
+    <>
+      <PropertyValue onClick={() => setOpen(true)}></PropertyValue>
       <ConfigModal
         onBack={() => setOpen(false)}
         open={open}
@@ -125,7 +126,7 @@ const MobileRichTextCell = ({
         onChange={onChange}
         rowId={rowId}
       />
-    </PropertyValue>
+    </>
   );
 };
 

diff --git a/tests/affine-mobile/e2e/settings.spec.ts b/tests/affine-mobile/e2e/settings.spec.ts
@@ -0,0 +1,60 @@
+import { test } from '@affine-test/kit/mobile';
+import { expect, type Page } from '@playwright/test';
+
+const openSettings = async (page: Page) => {
+  await page.getByTestId('settings-button').click();
+  await expect(page.getByRole('dialog')).toBeVisible();
+  await expect(page.locator('header:has-text("Settings")')).toBeVisible();
+};
+
+test('can open settings', async ({ page }) => {
+  await openSettings(page);
+});
+
+test('can change theme', async ({ page }) => {
+  await openSettings(page);
+  await page
+    .getByTestId('setting-row')
+    .filter({
+      hasText: 'Color mode',
+    })
+    .getByTestId('dropdown-select-trigger')
+    .click();
+
+  await expect(
+    page.getByRole('dialog').filter({
+      has: page.getByRole('menuitem', { name: 'Light' }),
+    })
+  ).toBeVisible();
+
+  await page.getByRole('menuitem', { name: 'Dark' }).click();
+
+  await expect(page.locator('html')).toHaveAttribute('data-theme', 'dark');
+});
+
+test('can close change theme popover by clicking outside', async ({ page }) => {
+  await openSettings(page);
+  await page
+    .getByTestId('setting-row')
+    .filter({
+      hasText: 'Color mode',
+    })
+    .getByTestId('dropdown-select-trigger')
+    .click();
+
+  const themePopover = page.getByRole('dialog').filter({
+    has: page.getByRole('menuitem', { name: 'Light' }),
+  });
+
+  await expect(themePopover).toBeVisible();
+
+  // get a mouse position that is 10px offset to the top of theme popover
+  // and click
+  const mousePosition = await themePopover.boundingBox();
+  if (!mousePosition) {
+    throw new Error('theme popover is not visible');
+  }
+  await page.mouse.click(mousePosition.x, mousePosition.y - 10);
+
+  await expect(themePopover).not.toBeVisible();
+});
diff --git a/tests/affine-mobile/playwright.config.ts b/tests/affine-mobile/playwright.config.ts
@@ -10,19 +10,21 @@ const config: PlaywrightTestConfig = {
   timeout: process.env.CI ? 60_000 : 30_000,
   outputDir: testResultDir,
   projects: [
-    {
-      name: 'Mobile Safari',
-      use: {
-        ...devices['iPhone 14'],
-      },
-    },
+    process.env.CI
+      ? {
+          name: 'Mobile Safari',
+          use: {
+            ...devices['iPhone 14'],
+          },
+        }
+      : undefined,
     {
       name: 'Mobile Chrome',
       use: {
         ...devices['Pixel 5'],
       },
     },
-  ],
+  ].filter(config => config !== undefined),
   expect: {
     timeout: process.env.CI ? 15_000 : 5_000,
   },