Browse Source

Merge branch 'dev/7.4.x' into fix/disable-logo-update-without-file

hikaruNAKANO 2 months ago
parent
commit
e68047f7dd

+ 2 - 7
apps/app/playwright/20-basic-features/access-to-page.spec.ts

@@ -1,11 +1,6 @@
-import { expect, type Page, test } from '@playwright/test';
+import { expect, test } from '@playwright/test';
 
-const appendTextToEditorUntilContains = async (page: Page, text: string) => {
-  await page.locator('.cm-content').fill(text);
-  await expect(page.getByTestId('page-editor-preview-body')).toContainText(
-    text,
-  );
-};
+import { appendTextToEditorUntilContains } from '../utils/AppendTextToEditorUntilContains';
 
 test('has title', async ({ page }) => {
   await page.goto('/Sandbox');

+ 2 - 7
apps/app/playwright/23-editor/saving.spec.ts

@@ -1,12 +1,7 @@
-import { expect, type Page, test } from '@playwright/test';
+import { expect, test } from '@playwright/test';
 import path from 'path';
 
-const appendTextToEditorUntilContains = async (page: Page, text: string) => {
-  await page.locator('.cm-content').fill(text);
-  await expect(page.getByTestId('page-editor-preview-body')).toContainText(
-    text,
-  );
-};
+import { appendTextToEditorUntilContains } from '../utils/AppendTextToEditorUntilContains';
 
 test('Successfully create page under specific path', async ({ page }) => {
   const newPagePath = '/child';

+ 77 - 0
apps/app/playwright/23-editor/vim-keymap.spec.ts

@@ -0,0 +1,77 @@
+import { expect, type Page, test } from '@playwright/test';
+
+import { appendTextToEditorUntilContains } from '../utils/AppendTextToEditorUntilContains';
+
+/**
+ * Tests for Vim keymap functionality in the editor
+ * @see https://github.com/growilabs/growi/issues/8814
+ * @see https://github.com/growilabs/growi/issues/10701
+ */
+
+const changeKeymap = async (page: Page, keymap: string) => {
+  // Open OptionsSelector
+  await expect(page.getByTestId('options-selector-btn')).toBeVisible();
+  await page.getByTestId('options-selector-btn').click();
+  await expect(page.getByTestId('options-selector-menu')).toBeVisible();
+
+  // Click keymap selection button to navigate to keymap selector
+  await expect(page.getByTestId('keymap_current_selection')).toBeVisible();
+  await page.getByTestId('keymap_current_selection').click();
+
+  // Select Vim keymap
+  await expect(page.getByTestId(`keymap_radio_item_${keymap}`)).toBeVisible();
+  await page.getByTestId(`keymap_radio_item_${keymap}`).click();
+
+  // Close OptionsSelector
+  await page.getByTestId('options-selector-btn').click();
+  await expect(page.getByTestId('options-selector-menu')).not.toBeVisible();
+};
+
+test.describe
+  .serial('Vim keymap mode', () => {
+    test.beforeEach(async ({ page }) => {
+      await page.goto('/Sandbox/vim-keymap-test-page');
+
+      // Open Editor
+      await expect(page.getByTestId('editor-button')).toBeVisible();
+      await page.getByTestId('editor-button').click();
+      await expect(page.locator('.cm-content')).toBeVisible();
+      await expect(page.getByTestId('grw-editor-navbar-bottom')).toBeVisible();
+    });
+
+    test('Insert mode should persist while typing multiple characters', async ({
+      page,
+    }) => {
+      const testText = 'Hello World';
+
+      // Change to Vim keymap
+      await changeKeymap(page, 'vim');
+
+      // Focus the editor
+      await page.locator('.cm-content').click();
+
+      // Enter insert mode
+      await page.keyboard.type('i');
+
+      // Append text
+      await appendTextToEditorUntilContains(page, testText);
+    });
+
+    test('Write command (:w) should save the page successfully', async ({
+      page,
+    }) => {
+      // Enter command mode
+      await page.keyboard.type(':');
+      await expect(page.locator('.cm-vim-panel')).toBeVisible();
+
+      // Type write command and execute
+      await page.keyboard.type('w');
+      await page.keyboard.press('Enter');
+
+      // Expect a success toaster to be displayed
+      await expect(page.locator('.Toastify__toast--success')).toBeVisible();
+
+      // Restore keymap to default
+      await changeKeymap(page, 'default');
+    });
+  });

+ 3 - 8
apps/app/playwright/23-editor/with-navigation.spec.ts

@@ -1,7 +1,9 @@
-import { expect, type Page, test } from '@playwright/test';
+import { expect, test } from '@playwright/test';
 import { readFileSync } from 'fs';
 import path from 'path';
 
+import { appendTextToEditorUntilContains } from '../utils/AppendTextToEditorUntilContains';
+
 /**
  * for the issues:
  * @see https://redmine.weseek.co.jp/issues/122040
@@ -61,13 +63,6 @@ test('should not be cleared and should prevent GrantSelector from modified', asy
   );
 });
 
-const appendTextToEditorUntilContains = async (page: Page, text: string) => {
-  await page.locator('.cm-content').fill(text);
-  await expect(page.getByTestId('page-editor-preview-body')).toContainText(
-    text,
-  );
-};
-
 /**
  * for the issue:
  * @see https://redmine.weseek.co.jp/issues/115285

+ 11 - 0
apps/app/playwright/utils/AppendTextToEditorUntilContains.ts

@@ -0,0 +1,11 @@
+import { expect, type Page } from '@playwright/test';
+
+export const appendTextToEditorUntilContains = async (
+  page: Page,
+  text: string,
+) => {
+  await page.locator('.cm-content').fill(text);
+  await expect(page.getByTestId('page-editor-preview-body')).toContainText(
+    text,
+  );
+};

+ 11 - 2
apps/app/src/client/components/PageEditor/EditorNavbarBottom/OptionsSelector.tsx

@@ -32,12 +32,16 @@ type RadioListItemProps = {
   icon?: React.ReactNode;
   text: string;
   checked?: boolean;
+  dataTestid?: string;
 };
 
 const RadioListItem = (props: RadioListItemProps): JSX.Element => {
   const { onClick, icon, text, checked } = props;
   return (
-    <li className="list-group-item border-0 d-flex align-items-center">
+    <li
+      className="list-group-item border-0 d-flex align-items-center"
+      data-testid={props.dataTestid}
+    >
       <input
         onClick={onClick}
         className="form-check-input me-3"
@@ -177,6 +181,7 @@ const KeymapSelector = memo(
                 icon={icon}
                 text={keymapLabel}
                 checked={keymapMode === selectedKeymapMode}
+                dataTestid={`keymap_radio_item_${keymapMode}`}
               />
             );
           })}
@@ -337,6 +342,7 @@ type ChangeStateButtonProps = {
   header: string;
   data: string;
   disabled?: boolean;
+  dataTestid?: string;
 };
 const ChangeStateButton = memo((props: ChangeStateButtonProps): JSX.Element => {
   const { onClick, header, data, disabled } = props;
@@ -346,6 +352,7 @@ const ChangeStateButton = memo((props: ChangeStateButtonProps): JSX.Element => {
       className="d-flex align-items-center btn btn-sm border-0 my-1"
       disabled={disabled}
       onClick={onClick}
+      data-testid={props.dataTestid}
     >
       <span className="ms-2 me-auto">{header}</span>
       <span className="text-muted d-flex align-items-center ms-2 me-1">
@@ -397,6 +404,7 @@ export const OptionsSelector = (): JSX.Element => {
       className=""
     >
       <DropdownToggle
+        data-testid="options-selector-btn"
         className={`btn btn-sm btn-outline-neutral-secondary d-flex align-items-center justify-content-center
               ${isDeviceLargerThanMd ? '' : 'border-0'}
               ${dropdownOpen ? 'active' : ''}
@@ -409,7 +417,7 @@ export const OptionsSelector = (): JSX.Element => {
           <></>
         )}
       </DropdownToggle>
-      <DropdownMenu container="body">
+      <DropdownMenu container="body" data-testid="options-selector-menu">
         {status === OptionsStatus.Home && (
           <div className="d-flex flex-column">
             <span className="text-muted ms-3">
@@ -426,6 +434,7 @@ export const OptionsSelector = (): JSX.Element => {
               onClick={() => setStatus(OptionsStatus.Keymap)}
               header={t('page_edit.keymap')}
               data={KEYMAP_LABEL_MAP[editorSettings.keymapMode ?? ''] ?? ''}
+              dataTestid="keymap_current_selection"
             />
             <hr className="my-1" />
             <ChangeStateButton

+ 100 - 1
apps/app/src/services/renderer/recommended-whitelist.spec.ts

@@ -1,4 +1,4 @@
-import { notDeepEqual } from 'assert';
+import assert from 'assert';
 
 import { attributes, tagNames } from './recommended-whitelist';
 
@@ -80,4 +80,103 @@ describe('recommended-whitelist', () => {
       'data-footnote-backref',
     ]);
   });
+
+  // Tests for restored semantic HTML tags
+  describe('semantic HTML tags restored from v6.3.5', () => {
+    test('.tagNames should include abbr tag', () => {
+      expect(tagNames).toContain('abbr');
+    });
+
+    test('.tagNames should include bdo tag', () => {
+      expect(tagNames).toContain('bdo');
+    });
+
+    test('.tagNames should include caption tag', () => {
+      expect(tagNames).toContain('caption');
+    });
+
+    test('.tagNames should include cite tag', () => {
+      expect(tagNames).toContain('cite');
+    });
+
+    test('.tagNames should include dfn tag', () => {
+      expect(tagNames).toContain('dfn');
+    });
+
+    test('.tagNames should include figure tag', () => {
+      expect(tagNames).toContain('figure');
+    });
+
+    test('.tagNames should include figcaption tag', () => {
+      expect(tagNames).toContain('figcaption');
+    });
+
+    test('.tagNames should include mark tag', () => {
+      expect(tagNames).toContain('mark');
+    });
+
+    test('.tagNames should include small tag', () => {
+      expect(tagNames).toContain('small');
+    });
+
+    test('.tagNames should include time tag', () => {
+      expect(tagNames).toContain('time');
+    });
+
+    test('.tagNames should include wbr tag', () => {
+      expect(tagNames).toContain('wbr');
+    });
+  });
+
+  describe('attributes for semantic HTML tags', () => {
+    test('.attributes should have abbr with title attribute', () => {
+      expect(attributes).not.toBeNull();
+      assert(attributes != null);
+      expect(Object.keys(attributes)).toContain('abbr');
+      expect(attributes.abbr).toContain('title');
+    });
+
+    test('.attributes should have bdo with dir attribute', () => {
+      expect(attributes).not.toBeNull();
+      assert(attributes != null);
+      expect(Object.keys(attributes)).toContain('bdo');
+      expect(attributes.bdo).toContain('dir');
+    });
+
+    test('.attributes should have dfn with title attribute', () => {
+      expect(attributes).not.toBeNull();
+      assert(attributes != null);
+      expect(Object.keys(attributes)).toContain('dfn');
+      expect(attributes.dfn).toContain('title');
+    });
+
+    test('.attributes should have time with datetime attribute', () => {
+      expect(attributes).not.toBeNull();
+      assert(attributes != null);
+      expect(Object.keys(attributes)).toContain('time');
+      expect(attributes.time).toContain('datetime');
+    });
+
+    test('.attributes should have empty arrays for tags without specific attributes', () => {
+      expect(attributes).not.toBeNull();
+
+      // Tags that should have empty attribute arrays
+      const tagsWithEmptyAttributes = [
+        'caption',
+        'cite',
+        'figure',
+        'figcaption',
+        'mark',
+        'small',
+        'wbr',
+      ];
+
+      tagsWithEmptyAttributes.forEach((tag) => {
+        assert(attributes != null);
+
+        expect(Object.keys(attributes)).toContain(tag);
+        expect(attributes[tag]).toEqual([]);
+      });
+    });
+  });
 });

+ 23 - 1
apps/app/src/services/renderer/recommended-whitelist.ts

@@ -47,21 +47,43 @@ relaxedSchemaAttributes.li = excludeRestrictedClassAttributes(
 export const tagNames: Array<string> = [
   ...(defaultSchema.tagNames ?? []),
   '-',
+  'abbr',
+  'bdo',
   'bdi',
   'button',
+  'caption',
+  'cite',
   'col',
   'colgroup',
   'data',
+  'dfn',
+  'figure',
+  'figcaption',
   'iframe',
-  'video',
+  'mark',
   'rb',
+  'small',
+  'time',
   'u',
+  'video',
+  'wbr',
 ];
 
 export const attributes: Attributes = deepmerge(relaxedSchemaAttributes, {
   a: ['target'],
+  abbr: ['title'],
+  bdo: ['dir'],
+  caption: [],
+  cite: [],
+  dfn: ['title'],
+  figure: [],
+  figcaption: [],
   iframe: ['allow', 'referrerpolicy', 'sandbox', 'src'],
+  mark: [],
+  small: [],
+  time: ['datetime'],
   video: ['controls', 'src', 'muted', 'preload', 'width', 'height', 'autoplay'],
+  wbr: [],
   // The special value 'data*' as a property name can be used to allow all data properties.
   // see: https://github.com/syntax-tree/hast-util-sanitize/
   '*': ['key', 'class', 'className', 'style', 'role', 'data*'],

+ 12 - 3
packages/editor/src/client/stores/use-editor-settings.ts

@@ -1,4 +1,4 @@
-import { useCallback, useEffect, useState } from 'react';
+import { useCallback, useEffect, useRef, useState } from 'react';
 import type { Extension } from '@codemirror/state';
 import { Prec } from '@codemirror/state';
 import {
@@ -93,12 +93,21 @@ const useKeymapExtension = (
     undefined,
   );
 
+  // Use ref for onSave to prevent keymap extension recreation on callback changes
+  // This is critical for Vim mode to preserve insert mode state
+  const onSaveRef = useRef(onSave);
+  useEffect(() => {
+    onSaveRef.current = onSave;
+  }, [onSave]);
+
   useEffect(() => {
     const settingKeyMap = async (name?: KeyMapMode) => {
-      setKeymapExtension(await getKeymap(name, onSave));
+      // Pass a stable wrapper function that delegates to the ref
+      const stableOnSave = () => onSaveRef.current?.();
+      setKeymapExtension(await getKeymap(name, stableOnSave));
     };
     settingKeyMap(keymapMode);
-  }, [keymapMode, onSave]);
+  }, [keymapMode]);
 
   useEffect(() => {
     if (keymapExtension == null) {