From 8caa9edebbd9fbf44f6385737476662d741a3d66 Mon Sep 17 00:00:00 2001 From: Je Xia Date: Wed, 22 Jul 2026 02:43:58 +0800 Subject: [PATCH] fix(diffs): Keep programmatic selections from opening selection action widget --- .../docs/app/(diffs)/docs/Editor/constants.ts | 5 +- apps/docs/app/(diffs)/docs/Editor/content.mdx | 7 +- packages/diffs/src/editor/editor.ts | 15 ++- .../diffs/test/editorSelectionAction.test.ts | 99 ++++++++++++++++++- 4 files changed, 117 insertions(+), 9 deletions(-) diff --git a/apps/docs/app/(diffs)/docs/Editor/constants.ts b/apps/docs/app/(diffs)/docs/Editor/constants.ts index 63a63a974..d6b538b7d 100644 --- a/apps/docs/app/(diffs)/docs/Editor/constants.ts +++ b/apps/docs/app/(diffs)/docs/Editor/constants.ts @@ -346,7 +346,7 @@ export const EDITOR_SELECTION_ACTION_EXAMPLE: PreloadFileOptions = { const editor = new Editor({ enabledSelectionAction: true, - // The popover appears automatically on selection (no icon, no extra click). + // The popover appears after a user-created ranged selection. renderSelectionAction: (context) => { const container = document.createElement('div'); const button = document.createElement('button'); @@ -952,7 +952,8 @@ interface EditorOptions { // (default: 'default' — both quotes and brackets) autoSurround?: 'default' | 'never' | 'brackets' | 'quotes' | 'languageDefined'; - // Show the floating Selection Action popover on selection (default: false) + // Show the floating Selection Action popover after a user selection. + // Programmatic setSelections/setState calls do not open it (default: false). enabledSelectionAction?: boolean; // Custom clipboard provider. diff --git a/apps/docs/app/(diffs)/docs/Editor/content.mdx b/apps/docs/app/(diffs)/docs/Editor/content.mdx index 2483e009a..b0b781cb4 100644 --- a/apps/docs/app/(diffs)/docs/Editor/content.mdx +++ b/apps/docs/app/(diffs)/docs/Editor/content.mdx @@ -393,9 +393,10 @@ where editing is rare. Selection Action is an opt-in edit mode feature for showing custom UI alongside the current selection—useful for quick transforms, refactor prompts, or other selection-scoped tools. Set `enabledSelectionAction: true` and return your UI -from `renderSelectionAction`; a floating popover holding your UI appears -automatically, anchored to the active selection. The popover can hold any number -of actions. +from `renderSelectionAction`; a floating popover holding your UI appears after +the user creates a ranged selection. Programmatic `setSelections` and `setState` +calls update the selection without opening the popover. The popover can hold any +number of actions. diff --git a/packages/diffs/src/editor/editor.ts b/packages/diffs/src/editor/editor.ts index d2f0b9f49..0371249aa 100644 --- a/packages/diffs/src/editor/editor.ts +++ b/packages/diffs/src/editor/editor.ts @@ -208,8 +208,8 @@ export interface EditorOptions { /** Per-language comment tokens used by the comment commands. */ languageCommentConfig?: LanguageConfigMap; /** - * Show a floating selection action popover anchored to the active selection, - * default is disabled. + * Show a floating selection action popover after a user-created selection, + * default is disabled. Programmatic selection updates do not open it. */ enabledSelectionAction?: boolean; /** @@ -336,6 +336,8 @@ export class Editor implements DiffsEditor { #markerRenderer?: MarkerRenderer; #searchPanel?: SearchPanelWidget; #selectionAction?: SelectionActionWidget; + // Programmatic ranges stay passive until the user interacts with the editor. + #canMountSelectionAction = true; #shouldIgnoreSelectionChange = false; // Set when the browser's native Selection is temporarily not the source of // truth for editor text selections. Select-all can only mirror rendered @@ -594,6 +596,7 @@ export class Editor implements DiffsEditor { if (this.#fileInstance === undefined || this.#textDocument === undefined) { throw new Error('Editor is not attached'); } + this.#canMountSelectionAction = false; this.#updateSelections(selections ?? []); // When a saved view is present, honor its scroll offsets exactly. Scrolling // the caret into view afterward would overwrite them whenever the caret @@ -640,6 +643,7 @@ export class Editor implements DiffsEditor { } return { direction, start, end }; }); + this.#canMountSelectionAction = false; this.#updateSelections(resolvedSelections); const primarySelection = resolvedSelections.at(-1); if (primarySelection !== undefined) { @@ -1365,6 +1369,7 @@ export class Editor implements DiffsEditor { this.#searchPanel = undefined; this.#selectionAction?.cleanup(); this.#selectionAction = undefined; + this.#canMountSelectionAction = true; } #invalidateOnAttach(): void { @@ -1688,6 +1693,7 @@ export class Editor implements DiffsEditor { contentEl, 'pointerdown', (e) => { + this.#canMountSelectionAction = true; // Any pointer press — mouse, touch, or pen — moves off the select-all // selection, so let selectionchange sync #selections from the new // native selection again. This must run before the mouse-only guard @@ -1780,6 +1786,7 @@ export class Editor implements DiffsEditor { if (!targetIsContentElement(e)) { return; } + this.#canMountSelectionAction = true; // A keystroke is the user acting on the select-all selection (deleting, // typing, moving); let selectionchange sync #selections again. this.#suppressNativeSelectionSync = false; @@ -2130,6 +2137,7 @@ export class Editor implements DiffsEditor { return; } + this.#canMountSelectionAction = true; this.#markerRenderer?.removePopover(); const selection = this.#spanLineSelection( lineIndex, @@ -4295,6 +4303,9 @@ export class Editor implements DiffsEditor { } if (this.#selectionAction === undefined) { + if (!this.#canMountSelectionAction) { + return; + } const getActiveSelection = (): EditorSelection => this.#selections?.at(-1) ?? primarySelection; const selectionActionElement = renderSelectionAction({ diff --git a/packages/diffs/test/editorSelectionAction.test.ts b/packages/diffs/test/editorSelectionAction.test.ts index aae8abb73..0a262e0f0 100644 --- a/packages/diffs/test/editorSelectionAction.test.ts +++ b/packages/diffs/test/editorSelectionAction.test.ts @@ -4,10 +4,14 @@ import { File } from '../src/components/File'; import { DEFAULT_THEMES } from '../src/constants'; import { Editor, type EditorOptions } from '../src/editor/editor'; import { PopoverManager } from '../src/editor/popover'; -import { DirectionBackward, getCaretPosition } from '../src/editor/selection'; +import { + DirectionBackward, + DirectionForward, + getCaretPosition, +} from '../src/editor/selection'; import type { SelectionActionContext } from '../src/editor/selectionAction'; import { disposeHighlighter } from '../src/highlighter/shared_highlighter'; -import type { FileContents, RenderRange } from '../src/types'; +import type { EditorSelection, FileContents, RenderRange } from '../src/types'; import { installDom, wait } from './domHarness'; afterAll(async () => { @@ -99,6 +103,17 @@ function findSelectionActionPopover(content: HTMLElement): HTMLElement { return popover; } +// setSelections supplies deterministic ranges in jsdom; the pointer gesture +// marks them as user-created so selection-action tests exercise the mount path. +function settleSelectionAction(content: HTMLElement): void { + content.dispatchEvent( + new PointerEvent('pointerdown', { button: 0, pointerType: 'mouse' }) + ); + document.dispatchEvent( + new PointerEvent('pointerup', { button: 0, pointerType: 'mouse' }) + ); +} + describe('Editor selection action', () => { // The popover element is created once when the selection settles and kept open // across selection changes, so its handlers must read the current primary @@ -127,6 +142,7 @@ describe('Editor selection action', () => { direction: 'forward', }, ]); + settleSelectionAction(content); // The selection grows on the same line; the popover stays open. editor.setSelections([ @@ -173,6 +189,7 @@ describe('Editor selection action', () => { direction: 'backward', }, ]); + settleSelectionAction(content); // The selection grows backward on the same line; the popover stays open. editor.setSelections([ @@ -217,6 +234,7 @@ describe('Editor selection action', () => { direction: 'backward', }, ]); + settleSelectionAction(content); const popover = findSelectionActionPopover(content); expect(popover.style.getPropertyValue('--popover-y-shift').trim()).toBe( @@ -248,6 +266,7 @@ describe('Editor selection action', () => { direction: 'forward', }, ]); + settleSelectionAction(content); const popover = findSelectionActionPopover(content); expect(popover.style.getPropertyValue('--popover-y-shift').trim()).toBe( @@ -280,6 +299,7 @@ describe('Editor selection action', () => { direction: 'backward', }, ]); + settleSelectionAction(content); const popover = findSelectionActionPopover(content); expect(popover.style.getPropertyValue('--popover-y-shift').trim()).toBe( @@ -312,6 +332,7 @@ describe('Editor selection action', () => { direction: 'forward', }, ]); + settleSelectionAction(content); const popover = findSelectionActionPopover(content); expect(popover.style.getPropertyValue('--popover-y-shift').trim()).toBe( @@ -438,6 +459,7 @@ describe('Editor selection action', () => { direction: 'backward', }, ]); + settleSelectionAction(content); // Sanity check the fallback candidate's row actually differs from the // head's, so the assertion below isn't vacuously true. @@ -571,6 +593,7 @@ describe('Editor selection action', () => { direction: 'backward', }, ]); + settleSelectionAction(content); const popover = findSelectionActionPopover(content); // The initial 20px height fits above the head row: top is 10px inside @@ -639,6 +662,7 @@ describe('Editor selection action', () => { direction: 'backward', }, ]); + settleSelectionAction(content); const popover = findSelectionActionPopover(content); // Preferred (head-anchored, placed above) must win: the fallback's @@ -820,6 +844,7 @@ describe('Editor selection action', () => { direction: 'forward', }, ]); + settleSelectionAction(content); expect(() => findSelectionActionPopover(content)).not.toThrow(); editor.setSelections([ @@ -960,6 +985,75 @@ describe('Editor selection action', () => { }); }); + test('setSelections does not mount a selection action', async () => { + let renderCount = 0; + const { cleanup, editor, content } = await createSelectionActionFixture( + 'hello world', + { + enabledSelectionAction: true, + renderSelectionAction() { + renderCount++; + return document.createElement('div'); + }, + } + ); + + try { + const start = { line: 0, character: 0 }; + const end = { line: 0, character: 5 }; + editor.setSelections([{ start, end, direction: 'forward' }]); + editor.setMarkers([]); + await wait(0); + + expect(editor.getState().selections).toEqual([ + { start, end, direction: DirectionForward }, + ]); + expect(renderCount).toBe(0); + expect( + (content.getRootNode() as ShadowRoot).querySelector( + '[data-selection-action-popover]' + ) + ).toBeNull(); + } finally { + cleanup(); + } + }); + + test('setState does not mount a selection action', async () => { + let renderCount = 0; + const { cleanup, editor, content } = await createSelectionActionFixture( + 'hello world', + { + enabledSelectionAction: true, + renderSelectionAction() { + renderCount++; + return document.createElement('div'); + }, + } + ); + + try { + const selection: EditorSelection = { + start: { line: 0, character: 0 }, + end: { line: 0, character: 5 }, + direction: DirectionForward, + }; + editor.setState({ selections: [selection] }); + editor.setMarkers([]); + await wait(0); + + expect(editor.getState().selections).toEqual([selection]); + expect(renderCount).toBe(0); + expect( + (content.getRootNode() as ShadowRoot).querySelector( + '[data-selection-action-popover]' + ) + ).toBeNull(); + } finally { + cleanup(); + } + }); + // Without `enabledSelectionAction`, a ranged selection renders nothing and the // consumer's callback is never invoked. test('renders no popover when the feature is disabled', async () => { @@ -982,6 +1076,7 @@ describe('Editor selection action', () => { direction: 'forward', }, ]); + settleSelectionAction(content); const root = content.getRootNode() as ShadowRoot; expect(root.querySelector('[data-selection-action-popover]')).toBeNull(); expect(rendered).toBe(false);