Skip to content

Commit ac15e31

Browse files
fix: prefer full decoration range (#2239)
* fix: prefer full decoration range * fix: consider docChanged flag * fix: add test/simplify logic with helper * fix: remove dead restore fallback in #collectDesiredState #resolveEffectiveRanges already calls restoreRangesFromPrevious with the same arguments — the second call was guaranteed to return the same empty result. Also adds the missing #setPreviousRanges call to clear stale ranges on this path. --------- Co-authored-by: Caio Pizzol <caio@harbourshare.com>
1 parent f4cb44d commit ac15e31

2 files changed

Lines changed: 255 additions & 125 deletions

File tree

packages/super-editor/src/core/presentation-editor/dom/DecorationBridge.test.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -873,6 +873,94 @@ describe('DecorationBridge', () => {
873873
expect(ranges[0].from).toBe(13);
874874
expect(ranges[0].to).toBe(19);
875875
});
876+
877+
it('prefers full restored span when plugin returns partial (e.g. after applying mark in long selection)', () => {
878+
const fullText = 'Hello world';
879+
const plugin = mutableExternalPlugin('focus');
880+
plugin.setDecorations([{ from: 1, to: 12, class: 'highlight-selection' }]);
881+
const state = mockStateWithDocText([plugin.plugin], fullText);
882+
883+
const ranges1 = bridge.collectDecorationRanges(state);
884+
expect(ranges1).toHaveLength(1);
885+
expect(ranges1[0].to - ranges1[0].from).toBe(fullText.length);
886+
887+
// This behavior is only expected right after a doc-changing transaction (e.g. applying a mark)
888+
bridge.recordTransaction({ docChanged: true, mapping: { map: (pos: number) => pos } } as unknown as Transaction);
889+
890+
// Simulate plugin returning only a prefix after mapping (e.g. mark applied in middle)
891+
plugin.setDecorations([{ from: 1, to: 6, class: 'highlight-selection' }]);
892+
const ranges2 = bridge.collectDecorationRanges(state);
893+
expect(ranges2).toHaveLength(1);
894+
// Full span restored by text so highlight does not partially vanish (not just partial 5 chars)
895+
expect(ranges2[0].to - ranges2[0].from).toBeGreaterThan(5);
896+
expect(ranges2[0].classes).toContain('highlight-selection');
897+
});
898+
899+
it('sync applies full span when plugin returns partial so highlight does not vanish', () => {
900+
const { index, addSpan, rebuild } = createIndex();
901+
addSpan(1, 6, 'Hello');
902+
const worldSpan = addSpan(6, 12, ' world');
903+
rebuild();
904+
905+
const { plugin, setDecorations } = mutableExternalPlugin('focus');
906+
const state = mockStateWithDocText([plugin], 'Hello world');
907+
setDecorations([{ from: 1, to: 12, class: 'highlight-selection' }]);
908+
bridge.collectDecorationRanges(state);
909+
bridge.sync(state, index);
910+
expect(worldSpan.classList.contains('highlight-selection')).toBe(true);
911+
912+
// Simulate mark application (doc change) that can cause mapping collapse for decorations
913+
bridge.recordTransaction({ docChanged: true, mapping: { map: (pos: number) => pos } } as unknown as Transaction);
914+
915+
setDecorations([{ from: 1, to: 6, class: 'highlight-selection' }]);
916+
bridge.sync(state, index);
917+
expect(worldSpan.classList.contains('highlight-selection')).toBe(true);
918+
});
919+
920+
it('returns narrow range as-is when plugin narrows after meta-only transaction (docChanged: false)', () => {
921+
const fullText = 'Hello world';
922+
const plugin = mutableExternalPlugin('focus');
923+
plugin.setDecorations([{ from: 1, to: 12, class: 'highlight-selection' }]);
924+
const state = mockStateWithDocText([plugin.plugin], fullText);
925+
926+
bridge.collectDecorationRanges(state);
927+
expect(bridge.collectDecorationRanges(state)).toHaveLength(1);
928+
929+
// Meta-only (e.g. setFocus with smaller range): no doc change
930+
bridge.recordTransaction({ docChanged: false, mapping: { map: (pos: number) => pos } } as unknown as Transaction);
931+
plugin.setDecorations([{ from: 1, to: 6, class: 'highlight-selection' }]);
932+
933+
const ranges = bridge.collectDecorationRanges(state);
934+
expect(ranges).toHaveLength(1);
935+
// Must not expand: narrow range comes back as-is (from 1 to 6 = 5 chars)
936+
expect(ranges[0].to - ranges[0].from).toBe(5);
937+
expect(ranges[0].from).toBe(1);
938+
expect(ranges[0].to).toBe(6);
939+
});
940+
941+
it('sync applies narrow range only when plugin narrows after meta-only transaction', () => {
942+
const { index, addSpan, rebuild } = createIndex();
943+
const helloSpan = addSpan(1, 6, 'Hello');
944+
const worldSpan = addSpan(6, 12, ' world');
945+
rebuild();
946+
947+
const { plugin, setDecorations } = mutableExternalPlugin('focus');
948+
const state = mockStateWithDocText([plugin], 'Hello world');
949+
setDecorations([{ from: 1, to: 12, class: 'highlight-selection' }]);
950+
bridge.collectDecorationRanges(state);
951+
bridge.sync(state, index);
952+
expect(helloSpan.classList.contains('highlight-selection')).toBe(true);
953+
expect(worldSpan.classList.contains('highlight-selection')).toBe(true);
954+
955+
// Meta-only: user narrowed selection (e.g. setFocus(1, 6))
956+
bridge.recordTransaction({ docChanged: false, mapping: { map: (pos: number) => pos } } as unknown as Transaction);
957+
setDecorations([{ from: 1, to: 6, class: 'highlight-selection' }]);
958+
bridge.sync(state, index);
959+
960+
// Narrow range only: first span keeps class, second loses it
961+
expect(helloSpan.classList.contains('highlight-selection')).toBe(true);
962+
expect(worldSpan.classList.contains('highlight-selection')).toBe(false);
963+
});
876964
});
877965

878966
// -----------------------------------------------------------------------

0 commit comments

Comments
 (0)