Skip to content

Commit 0a4b0b3

Browse files
authored
fix(superdoc): restore find input focus after match navigation (SD-3045) (#3240)
* fix(superdoc): restore find input focus after match navigation (SD-3045) The Search extension's goToSearchResult calls editor.view.focus() so the new selection is visible. When the user pressed Enter in the built-in find input, the synchronous focus steal blurred the input — subsequent Enter keystrokes were swallowed by the ProseMirror editor, splitting paragraphs at the match position, invalidating the search session, and resetting the active match index. The user had to click back into the input between every navigation. Re-focus the find input synchronously after goNext / goPrev so repeated Enter / Shift+Enter keeps advancing through matches and the editor never receives the keystroke. * fix(superdoc): scroll cross-page search matches into view reliably (SD-3045) Pressing Enter or clicking next/prev on a search match that lives on a different page often left the match just below the visible area, slightly above the viewport, or didn't scroll at all. Three races were colluding: 1. The Vue surface lives in the document's normal flow, so any focus or .select() on its input or buttons triggers the browser's scroll-element-into-view behaviour and snaps the document back to the find bar — undoing the goNext scroll. Drop .select() (cmd+A still works), focus with preventScroll: true, and pin every scrollable ancestor's scrollTop across the focus call. Route the button clicks through the same goNext/goPrev → focusFindInput path so all three navigation entry points (Enter, Shift+Enter, button click) have the same focus-restore guarantee. 2. PresentationEditor.scrollToPosition runs after dispatch(setSelection), but the selectionUpdate emitted by that dispatch sets #shouldScrollSelectionIntoView = true. The next #updateSelection then calls #scrollActiveEndIntoView and snaps the caret to its minimum-visibility position (often 20px from the viewport edge), visibly displacing the match. Consume the flag inside scrollToPosition. 3. A later selectionUpdate (focus blur as the user moves focus back to the find input, or async PM events) re-sets the flag to true after we consumed it, and the RAF-deferred #updateSelection scrolls the caret again. Re-assert the scrollIntoView on the next animation frame so any such late displacement is corrected; the no-op case is cheap. Tests cover all three: Enter / Shift+Enter / button-click focus restore use { preventScroll: true } on the find input. Manual repro on Luccas's finding1.docx fixture: 4 'titlePg' matches across 3 pages — every click now lands with the match in the centre of the viewport. * fix(superdoc): make search highlight win over inline background-color (SD-3045) Search matches were invisible on runs whose source rPr carried a highlight mark (e.g. `<w:highlight w:val="white"/>`). The DomPainter writes `style.backgroundColor = run.highlight` inline on the same span the DecorationBridge later tags with `.ProseMirror-search-match`, and inline styles win over class selectors — so the find colour was painted over. Add `!important` to both search-match background rules (the `.superdoc` scope used in presentation mode and the `.sd-editor-scoped` scope used by the hidden editor) so the transient find colour overrides any source-doc highlight while a search session is live. The class is only present during a search; clearing it restores the original highlight. Repro: load `basic/advanced-text.docx` from the corpus, search for "Oscar". 5 of 8 matches sit in runs imported as `highlight: { color: '#FFFFFF' }` and had no visible find highlight pre-fix. All 8 are now correctly coloured. Regression test asserts !important is present in both CSS files plus a JSDOM specificity sanity check showing the rule wins over an inline white.
1 parent d4d2eb1 commit 0a4b0b3

6 files changed

Lines changed: 477 additions & 11 deletions

File tree

packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,11 +432,15 @@ https://github.com/ProseMirror/prosemirror-tables/blob/master/demo/index.html
432432
margin-top: -5px;
433433
}
434434

435+
/* AIDEV-NOTE: SD-3045. `!important` so the transient search highlight wins
436+
* over any inline `style.backgroundColor` painted from a source-doc highlight
437+
* mark on the same span. See packages/superdoc/src/assets/styles/elements/superdoc.css
438+
* for the full rationale. */
435439
.sd-editor-scoped .ProseMirror-search-match {
436-
background-color: #ffff0054;
440+
background-color: #ffff0054 !important;
437441
}
438442
.sd-editor-scoped .ProseMirror-active-search-match {
439-
background-color: #ff6a0054;
443+
background-color: #ff6a0054 !important;
440444
}
441445
.sd-editor-scoped .ProseMirror span.sd-custom-selection::selection {
442446
background: transparent;

packages/super-editor/src/editors/v1/core/presentation-editor/PresentationEditor.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3534,7 +3534,37 @@ export class PresentationEditor extends EventEmitter {
35343534
if (pageEl) {
35353535
// Find the specific element containing this position for precise centering
35363536
const targetEl = this.#findElementAtPosition(pageEl, clampedPos);
3537-
(targetEl ?? pageEl).scrollIntoView({ block, inline: 'nearest', behavior });
3537+
const elToScroll = targetEl ?? pageEl;
3538+
elToScroll.scrollIntoView({ block, inline: 'nearest', behavior });
3539+
// AIDEV-NOTE: SD-3045. Search nav (and any other caller of
3540+
// scrollToPosition) places the viewport intentionally — usually
3541+
// centring the match. The next #updateSelection that runs as part
3542+
// of the dispatched setSelection transaction would otherwise call
3543+
// #scrollActiveEndIntoView and re-scroll the caret to its minimal
3544+
// visible position (often the top of the viewport), undoing our
3545+
// centring. Consume the pending scroll-into-view request so that
3546+
// selection sync renders the caret overlay without moving the
3547+
// scroll back. Other selection updates (Shift+Arrow, typing) re-set
3548+
// this flag themselves before they need scroll, so this consume is
3549+
// safe.
3550+
this.#shouldScrollSelectionIntoView = false;
3551+
// Re-assert the scroll on the next animation frame. The flag we
3552+
// cleared above defends against handleSelection that has already
3553+
// run, but a *later* selectionUpdate (e.g. focus blur fired when
3554+
// the user moves focus back to the find input) re-sets the flag to
3555+
// true before the RAF-scheduled #updateSelection fires, and that
3556+
// pass scrolls the caret to its minimal-visibility position —
3557+
// visibly snapping the match out of view. Re-running scrollIntoView
3558+
// on the same element a frame later overrides that snap; the no-op
3559+
// case (no late scroll happened) just re-centres the same element
3560+
// and is cheap.
3561+
const win = this.#visibleHost.ownerDocument?.defaultView;
3562+
if (win) {
3563+
win.requestAnimationFrame(() => {
3564+
elToScroll.scrollIntoView({ block, inline: 'nearest', behavior });
3565+
this.#shouldScrollSelectionIntoView = false;
3566+
});
3567+
}
35383568
return true;
35393569
}
35403570
}
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { readFileSync } from 'node:fs';
3+
import { fileURLToPath } from 'node:url';
4+
import { dirname, join } from 'node:path';
5+
6+
const __dirname = dirname(fileURLToPath(import.meta.url));
7+
8+
// SD-3045 (cross-package CSS invariant). The DomPainter writes
9+
// `style.backgroundColor = run.highlight` inline on the same span the search
10+
// DecorationBridge tags with `.ProseMirror-search-match`. Without `!important`,
11+
// the inline style wins and the search highlight is invisible on every run
12+
// whose source rPr carries a highlight mark (e.g. `<w:highlight w:val="white"/>`).
13+
// These tests guard the two CSS sites that paint the transient search colour.
14+
15+
const repoRoot = join(__dirname, '..', '..', '..', '..', '..', '..');
16+
17+
const superdocCss = readFileSync(
18+
join(repoRoot, 'packages', 'superdoc', 'src', 'assets', 'styles', 'elements', 'superdoc.css'),
19+
'utf8',
20+
);
21+
22+
const editorScopedCss = readFileSync(
23+
join(repoRoot, 'packages', 'super-editor', 'src', 'editors', 'v1', 'assets', 'styles', 'elements', 'prosemirror.css'),
24+
'utf8',
25+
);
26+
27+
const extractRuleBody = (css, selector) => {
28+
const idx = css.indexOf(selector);
29+
if (idx === -1) return null;
30+
const open = css.indexOf('{', idx);
31+
const close = css.indexOf('}', open);
32+
if (open === -1 || close === -1) return null;
33+
return css.slice(open + 1, close);
34+
};
35+
36+
describe('search-match CSS precedence (SD-3045)', () => {
37+
describe('packages/superdoc/src/assets/styles/elements/superdoc.css', () => {
38+
it('`.superdoc .ProseMirror-search-match` background uses !important', () => {
39+
const body = extractRuleBody(superdocCss, '.superdoc .ProseMirror-search-match');
40+
expect(body, '.superdoc .ProseMirror-search-match rule must exist').not.toBeNull();
41+
expect(body).toMatch(/background\s*:[^;]*!important/);
42+
});
43+
44+
it('`.superdoc .ProseMirror-active-search-match` background uses !important', () => {
45+
const body = extractRuleBody(superdocCss, '.superdoc .ProseMirror-active-search-match');
46+
expect(body, '.superdoc .ProseMirror-active-search-match rule must exist').not.toBeNull();
47+
expect(body).toMatch(/background\s*:[^;]*!important/);
48+
});
49+
});
50+
51+
describe('packages/super-editor/.../prosemirror.css', () => {
52+
it('`.sd-editor-scoped .ProseMirror-search-match` background-color uses !important', () => {
53+
const body = extractRuleBody(editorScopedCss, '.sd-editor-scoped .ProseMirror-search-match');
54+
expect(body, '.sd-editor-scoped .ProseMirror-search-match rule must exist').not.toBeNull();
55+
expect(body).toMatch(/background-color\s*:[^;]*!important/);
56+
});
57+
58+
it('`.sd-editor-scoped .ProseMirror-active-search-match` background-color uses !important', () => {
59+
const body = extractRuleBody(editorScopedCss, '.sd-editor-scoped .ProseMirror-active-search-match');
60+
expect(body, '.sd-editor-scoped .ProseMirror-active-search-match rule must exist').not.toBeNull();
61+
expect(body).toMatch(/background-color\s*:[^;]*!important/);
62+
});
63+
});
64+
65+
describe('JSDOM specificity sanity check', () => {
66+
it('class-level `background !important` beats inline `style="background-color: white"`', () => {
67+
const styleEl = document.createElement('style');
68+
styleEl.textContent = `.search-test { background: rgba(255, 213, 0, 0.4) !important; }`;
69+
document.head.appendChild(styleEl);
70+
71+
const span = document.createElement('span');
72+
span.className = 'search-test';
73+
span.setAttribute('style', 'background-color: rgb(255, 255, 255);');
74+
document.body.appendChild(span);
75+
76+
const bg = getComputedStyle(span).backgroundColor;
77+
78+
styleEl.remove();
79+
span.remove();
80+
81+
expect(bg).toBe('rgba(255, 213, 0, 0.4)');
82+
});
83+
84+
it('without !important, inline `background-color: white` overrides class background', () => {
85+
const styleEl = document.createElement('style');
86+
styleEl.textContent = `.search-test-noimp { background: rgba(255, 213, 0, 0.4); }`;
87+
document.head.appendChild(styleEl);
88+
89+
const span = document.createElement('span');
90+
span.className = 'search-test-noimp';
91+
span.setAttribute('style', 'background-color: rgb(255, 255, 255);');
92+
document.body.appendChild(span);
93+
94+
const bg = getComputedStyle(span).backgroundColor;
95+
96+
styleEl.remove();
97+
span.remove();
98+
99+
expect(bg).toBe('rgb(255, 255, 255)');
100+
});
101+
});
102+
});

packages/superdoc/src/assets/styles/elements/superdoc.css

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,21 @@
4040

4141
/* --- Search highlights (transient, used in both editor and presentation mode) --- */
4242

43+
/* AIDEV-NOTE: SD-3045. The DomPainter writes `style.backgroundColor = run.highlight`
44+
* inline on the same span the search DecorationBridge tags. Inline styles win over
45+
* class selectors, so on every run whose source rPr carries a highlight mark
46+
* (e.g. `<w:highlight w:val="white"/>`), the search highlight was invisible —
47+
* the inline background painted over the class's transient yellow/orange.
48+
* `!important` is the desired semantic here: while a search session is live, the
49+
* find-match colour must override any source-doc highlight. The class is only
50+
* present during a search; clearing the session removes it and the original
51+
* highlight is restored. */
4352
.superdoc .ProseMirror-search-match {
44-
background: var(--sd-ui-search-match-bg);
53+
background: var(--sd-ui-search-match-bg) !important;
4554
}
4655

4756
.superdoc .ProseMirror-active-search-match {
48-
background: var(--sd-ui-search-match-active-bg);
57+
background: var(--sd-ui-search-match-active-bg) !important;
4958
}
5059

5160
/* Contained Mode - fixed-height container embedding with internal scrolling */

0 commit comments

Comments
 (0)