-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(web-shell): render a plain textarea composer on touch devices #7587
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
71684a6
ed85222
8eebb70
e0b3014
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,6 @@ | ||||||||||||||||||||||||||||||||||||||||||
| // @vitest-environment jsdom | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| import { act } from 'react'; | ||||||||||||||||||||||||||||||||||||||||||
| import { act, createRef } from 'react'; | ||||||||||||||||||||||||||||||||||||||||||
| import { createRoot, type Root } from 'react-dom/client'; | ||||||||||||||||||||||||||||||||||||||||||
| import { afterEach, describe, expect, it, vi } from 'vitest'; | ||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -11,7 +11,10 @@ import { | |||||||||||||||||||||||||||||||||||||||||
| type WebShellCustomization, | ||||||||||||||||||||||||||||||||||||||||||
| } from '../customization'; | ||||||||||||||||||||||||||||||||||||||||||
| import { I18nProvider } from '../i18n'; | ||||||||||||||||||||||||||||||||||||||||||
| import type { SlashMenuState } from '../hooks/useComposerCore'; | ||||||||||||||||||||||||||||||||||||||||||
| import type { | ||||||||||||||||||||||||||||||||||||||||||
| MobileComposerBackend, | ||||||||||||||||||||||||||||||||||||||||||
| SlashMenuState, | ||||||||||||||||||||||||||||||||||||||||||
| } from '../hooks/useComposerCore'; | ||||||||||||||||||||||||||||||||||||||||||
| import { ChatEditor, type ComposerToolbarAction } from './ChatEditor'; | ||||||||||||||||||||||||||||||||||||||||||
| import { WebShellPortalRootContext } from '../portalRoot'; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -28,6 +31,8 @@ const composerCoreState = vi.hoisted(() => ({ | |||||||||||||||||||||||||||||||||||||||||
| slashMenu: null as SlashMenuState | null, | ||||||||||||||||||||||||||||||||||||||||||
| focus: vi.fn(), | ||||||||||||||||||||||||||||||||||||||||||
| closeSlashMenu: vi.fn(), | ||||||||||||||||||||||||||||||||||||||||||
| mobileComposer: null as unknown, | ||||||||||||||||||||||||||||||||||||||||||
| openHistorySearch: vi.fn(), | ||||||||||||||||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| Object.defineProperty(window, 'matchMedia', { | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -48,6 +53,7 @@ vi.mock('../hooks/useComposerCore', async (importOriginal) => { | |||||||||||||||||||||||||||||||||||||||||
| useComposerCore: () => ({ | ||||||||||||||||||||||||||||||||||||||||||
| containerRef: React.createRef<HTMLDivElement>(), | ||||||||||||||||||||||||||||||||||||||||||
| viewRef: { current: null }, | ||||||||||||||||||||||||||||||||||||||||||
| mobileComposer: composerCoreState.mobileComposer, | ||||||||||||||||||||||||||||||||||||||||||
| focus: composerCoreState.focus, | ||||||||||||||||||||||||||||||||||||||||||
| submitText: vi.fn(), | ||||||||||||||||||||||||||||||||||||||||||
| clearText: vi.fn(), | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -88,7 +94,7 @@ vi.mock('../hooks/useComposerCore', async (importOriginal) => { | |||||||||||||||||||||||||||||||||||||||||
| searchActiveIndex: 0, | ||||||||||||||||||||||||||||||||||||||||||
| searchInputRef: React.createRef<HTMLInputElement>(), | ||||||||||||||||||||||||||||||||||||||||||
| searchUiRef: React.createRef<HTMLDivElement>(), | ||||||||||||||||||||||||||||||||||||||||||
| openHistorySearch: vi.fn(), | ||||||||||||||||||||||||||||||||||||||||||
| openHistorySearch: composerCoreState.openHistorySearch, | ||||||||||||||||||||||||||||||||||||||||||
| closeSearch: vi.fn(), | ||||||||||||||||||||||||||||||||||||||||||
| submitSearchMatch: vi.fn(), | ||||||||||||||||||||||||||||||||||||||||||
| handleSearchKeyDown: vi.fn(), | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -132,6 +138,8 @@ afterEach(() => { | |||||||||||||||||||||||||||||||||||||||||
| composerCoreState.slashMenu = null; | ||||||||||||||||||||||||||||||||||||||||||
| composerCoreState.focus.mockReset(); | ||||||||||||||||||||||||||||||||||||||||||
| composerCoreState.closeSlashMenu.mockReset(); | ||||||||||||||||||||||||||||||||||||||||||
| composerCoreState.mobileComposer = null; | ||||||||||||||||||||||||||||||||||||||||||
| composerCoreState.openHistorySearch.mockReset(); | ||||||||||||||||||||||||||||||||||||||||||
| for (const { root, container, portalRoot } of mounted.splice(0)) { | ||||||||||||||||||||||||||||||||||||||||||
| act(() => root.unmount()); | ||||||||||||||||||||||||||||||||||||||||||
| container.remove(); | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -727,3 +735,88 @@ describe('ChatEditor slash command popovers', () => { | |||||||||||||||||||||||||||||||||||||||||
| expect(composerCoreState.closeSlashMenu).not.toHaveBeenCalled(); | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| describe('ChatEditor mobile composer quick actions', () => { | ||||||||||||||||||||||||||||||||||||||||||
| const originalMaxTouchPoints = Object.getOwnPropertyDescriptor( | ||||||||||||||||||||||||||||||||||||||||||
| Navigator.prototype, | ||||||||||||||||||||||||||||||||||||||||||
| 'maxTouchPoints', | ||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| function withTouchDevice(run: () => void) { | ||||||||||||||||||||||||||||||||||||||||||
| Object.defineProperty(navigator, 'maxTouchPoints', { | ||||||||||||||||||||||||||||||||||||||||||
| value: 5, | ||||||||||||||||||||||||||||||||||||||||||
| configurable: true, | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||
| run(); | ||||||||||||||||||||||||||||||||||||||||||
| } finally { | ||||||||||||||||||||||||||||||||||||||||||
| if (originalMaxTouchPoints) { | ||||||||||||||||||||||||||||||||||||||||||
| Object.defineProperty( | ||||||||||||||||||||||||||||||||||||||||||
| Navigator.prototype, | ||||||||||||||||||||||||||||||||||||||||||
| 'maxTouchPoints', | ||||||||||||||||||||||||||||||||||||||||||
| originalMaxTouchPoints, | ||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+752
to
+759
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The Currently no test is affected (this describe block is the last in the file, and vitest isolates test files), but adding
Suggested change
— qwen3.7-max via Qwen Code /review |
||||||||||||||||||||||||||||||||||||||||||
| delete (navigator as unknown as Record<string, unknown>)[ | ||||||||||||||||||||||||||||||||||||||||||
| 'maxTouchPoints' | ||||||||||||||||||||||||||||||||||||||||||
| ]; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| function mobileComposerStub(): MobileComposerBackend { | ||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||
| textareaRef: createRef<HTMLTextAreaElement>(), | ||||||||||||||||||||||||||||||||||||||||||
| value: '', | ||||||||||||||||||||||||||||||||||||||||||
| onChange: vi.fn(), | ||||||||||||||||||||||||||||||||||||||||||
| onPaste: vi.fn(), | ||||||||||||||||||||||||||||||||||||||||||
| placeholder: '', | ||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| function openQuickActions(container: HTMLElement) { | ||||||||||||||||||||||||||||||||||||||||||
| const toggle = container.querySelector<HTMLButtonElement>( | ||||||||||||||||||||||||||||||||||||||||||
| 'button[aria-label="more actions"]', | ||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||
| expect(toggle).not.toBeNull(); | ||||||||||||||||||||||||||||||||||||||||||
| act(() => toggle!.click()); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| it('maps the history quick action to the search UI on the mobile composer', () => { | ||||||||||||||||||||||||||||||||||||||||||
| withTouchDevice(() => { | ||||||||||||||||||||||||||||||||||||||||||
| composerCoreState.mobileComposer = mobileComposerStub(); | ||||||||||||||||||||||||||||||||||||||||||
| const container = renderChatEditor({}); | ||||||||||||||||||||||||||||||||||||||||||
| openQuickActions(container); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const historyButton = Array.from( | ||||||||||||||||||||||||||||||||||||||||||
| container.querySelectorAll('button'), | ||||||||||||||||||||||||||||||||||||||||||
| ).find((button) => button.textContent === 'Question history'); | ||||||||||||||||||||||||||||||||||||||||||
| expect(historyButton).not.toBeUndefined(); | ||||||||||||||||||||||||||||||||||||||||||
| act(() => historyButton!.click()); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| expect(composerCoreState.openHistorySearch).toHaveBeenCalledTimes(1); | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| it('hides the keyboard shortcut hints grid on the mobile composer', () => { | ||||||||||||||||||||||||||||||||||||||||||
| withTouchDevice(() => { | ||||||||||||||||||||||||||||||||||||||||||
| composerCoreState.mobileComposer = mobileComposerStub(); | ||||||||||||||||||||||||||||||||||||||||||
| const mobileContainer = renderChatEditor({}); | ||||||||||||||||||||||||||||||||||||||||||
| openQuickActions(mobileContainer); | ||||||||||||||||||||||||||||||||||||||||||
| expect( | ||||||||||||||||||||||||||||||||||||||||||
| Array.from(mobileContainer.querySelectorAll('button')).some( | ||||||||||||||||||||||||||||||||||||||||||
| (button) => button.textContent === 'Tab', | ||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||
| ).toBe(false); | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| composerCoreState.mobileComposer = null; | ||||||||||||||||||||||||||||||||||||||||||
| const desktopContainer = renderChatEditor({}); | ||||||||||||||||||||||||||||||||||||||||||
| openQuickActions(desktopContainer); | ||||||||||||||||||||||||||||||||||||||||||
| expect( | ||||||||||||||||||||||||||||||||||||||||||
| Array.from(desktopContainer.querySelectorAll('button')).some( | ||||||||||||||||||||||||||||||||||||||||||
| (button) => button.textContent === 'Tab', | ||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||
| ).toBe(true); | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1083,10 +1083,14 @@ function QuickActionsPanel({ | |
| actions, | ||
| onRun, | ||
| onPressKey, | ||
| showKeyHints = true, | ||
| }: { | ||
| actions: readonly QuickActionItem[]; | ||
| onRun: (action: QuickActionItem) => void; | ||
| onPressKey: (item: QuickKeyItem) => void; | ||
| // The keyboard shortcut grid is pointless without a hardware keyboard, so | ||
| // the mobile textarea backend hides it. | ||
| showKeyHints?: boolean; | ||
| }) { | ||
| const { t } = useI18n(); | ||
|
|
||
|
|
@@ -1110,20 +1114,22 @@ function QuickActionsPanel({ | |
| </button> | ||
| ))} | ||
| </div> | ||
| <div className={styles.quickKeysGrid}> | ||
| {QUICK_KEY_ITEMS.map((item) => ( | ||
| <button | ||
| key={item.id} | ||
| type="button" | ||
| className={styles.quickKey} | ||
| title={t(item.descriptionKey)} | ||
| onMouseDown={(event) => event.preventDefault()} | ||
| onClick={() => onPressKey(item)} | ||
| > | ||
| <span className={styles.quickKeyLabel}>{item.label}</span> | ||
| </button> | ||
| ))} | ||
| </div> | ||
| {showKeyHints && ( | ||
| <div className={styles.quickKeysGrid}> | ||
| {QUICK_KEY_ITEMS.map((item) => ( | ||
| <button | ||
| key={item.id} | ||
| type="button" | ||
| className={styles.quickKey} | ||
| title={t(item.descriptionKey)} | ||
| onMouseDown={(event) => event.preventDefault()} | ||
| onClick={() => onPressKey(item)} | ||
| > | ||
| <span className={styles.quickKeyLabel}>{item.label}</span> | ||
| </button> | ||
| ))} | ||
| </div> | ||
| )} | ||
| </div> | ||
| </div> | ||
| ); | ||
|
|
@@ -1503,6 +1509,15 @@ export const ChatEditor = memo( | |
| ); | ||
| const dispatchComposerKey = useCallback( | ||
| (event: QuickKeyItem['event']) => { | ||
| if (core.mobileComposer) { | ||
| // No CodeMirror to dispatch into. History search is the one key | ||
| // action with a non-keyboard equivalent; the rest are hidden on | ||
| // the textarea backend. | ||
| if (event.ctrlKey && event.key === 'r') { | ||
| core.searchState.openHistorySearch(); | ||
| } | ||
| return; | ||
| } | ||
|
Comment on lines
+1512
to
+1520
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] The Consider adding a DOM test that mounts the mobile composer, calls — qwen3.7-max via Qwen Code /review
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in ed85222 — ChatEditor-level tests with the mocked core exposing 中文:已在 ed85222 补测——ChatEditor 层用 mock core 暴露 |
||
| const view = core.viewRef.current; | ||
| if (!view) return; | ||
| view.focus(); | ||
|
|
@@ -2013,7 +2028,29 @@ export const ChatEditor = memo( | |
| ! | ||
| </span> | ||
| )} | ||
| <div ref={core.containerRef} data-web-shell-composer-editor /> | ||
| {core.mobileComposer ? ( | ||
| // Touch devices get a plain textarea instead of CodeMirror: | ||
| // mobile virtual keyboards and IMEs interact poorly with the | ||
| // contenteditable editor (#5958). Enter inserts a newline | ||
| // natively; submission goes through the Send button. | ||
| <textarea | ||
| ref={core.mobileComposer.textareaRef} | ||
| className={styles.mobileTextarea} | ||
| value={core.mobileComposer.value} | ||
| onChange={core.mobileComposer.onChange} | ||
| onPaste={core.mobileComposer.onPaste} | ||
| placeholder={core.mobileComposer.placeholder} | ||
| disabled={core.disabled} | ||
| rows={1} | ||
| enterKeyHint="enter" | ||
| autoCapitalize="off" | ||
| autoCorrect="off" | ||
| spellCheck={false} | ||
| data-web-shell-composer-editor | ||
| /> | ||
| ) : ( | ||
| <div ref={core.containerRef} data-web-shell-composer-editor /> | ||
| )} | ||
| </div> | ||
| <div ref={toolbarRef} className={styles.toolbar}> | ||
| <div ref={toolbarLeadingRef} className={styles.toolbarLeading}> | ||
|
|
@@ -2468,6 +2505,7 @@ export const ChatEditor = memo( | |
| actions={quickActions} | ||
| onRun={runQuickAction} | ||
| onPressKey={pressQuickKey} | ||
| showKeyHints={!core.mobileComposer} | ||
| /> | ||
| )} | ||
| </div> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion]
max-height: 300pxis dead CSS — no mechanism grows the textarea betweenmin-height(44px) andmax-height. The textarea hasrows={1},resize: none, and no JavaScript auto-grow (scrollHeight/autoGrowabsent from the entire diff). A mobile user typing a multi-line message sees only ~1.5 lines at a time with internal scrolling.Consider adding an auto-grow effect in
handleMobileChangeor auseEffectwatchingmobileText:— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in ed85222. Added an auto-grow effect watching the draft:
height = min(scrollHeight, computed max-height), so--chat-editor-input-max-heightoverrides stay authoritative and the CSS cap is no longer dead. The mobile e2e spec now asserts the textarea's bounding box grows across newlines.中文:已在 ed85222 修复——新增随草稿变化的 auto-grow effect,上限取计算样式的 max-height(CSS 变量覆盖仍生效);移动 e2e 增加了多行输入后高度增长的断言。