Skip to content

Commit fd1a740

Browse files
feat(super-editor): native LTR/RTL paragraph direction toolbar (#3226)
* feat(super-editor): native LTR/RTL paragraph direction toolbar Add ProseMirror commands `setParagraphDirection` / `clearParagraphDirection`, toolbar buttons next to the indent controls, and `Mod-Alt-Shift-L/R` keymaps. Buttons highlight to reflect the current paragraph's direction via the headless toolbar registry. Closes #3219. * chore(super-editor): drop LTR/RTL keymap, ship buttons only Matches Google Docs behavior — no default keyboard shortcut for paragraph direction. The 4-key Mod-Alt-Shift-L/R chord (added to avoid colliding with the existing Mod-Shift-L/R alignment binding) was awkward enough that any future shortcut should be a deliberate single-key toggle. * fix(super-editor): direction-rtl was silently applying LTR via no-payload path The `direction-ltr` / `direction-rtl` registry entries delegated to `createDirectCommandExecute('setParagraphDirection')`. Headless callers that invoke `controller.execute('direction-rtl')` without a payload ended up calling `editor.commands.setParagraphDirection()` with no arguments — which defaulted `direction` to `undefined`, evaluated `direction === 'rtl'` as false, and wrote `rightToLeft: false`. The RTL toolbar id silently performed an LTR write. Fix in two layers: - A dedicated `createParagraphDirectionExecute(direction)` helper that bakes the fixed `{ direction, alignmentPolicy: 'matchDirection' }` payload into the registry execute so callers don't need to know the direction is encoded in the command id. - Guard the command itself so a missing direction is a no-op rather than a silent LTR write — defense in depth for any other generic command-by-name pathway. * fix(super-editor): ltr direction deletes pPr.rightToLeft instead of writing false Writing `rightToLeft: false` round-trips as `<w:bidi w:val="0"/>` on export (direct formatting that overrides any inherited style direction), so clicking LTR on a vanilla paragraph injected `<w:bidi w:val="0"/>` into the DOCX even though nothing semantically changed. Now LTR deletes the property, matching `clearParagraphDirection` and leaving vanilla paragraphs untouched. Replaces a previous test that locked in the buggy behavior; adds a regression test for the vanilla-paragraph no-op case. * fix(super-editor): address PR #3226 review feedback Three issues caught in review: 1. Clicking LTR on a paragraph that inherits `rightToLeft: true` from its style was a silent no-op. The previous fix only deleted the inline override; with no inline value to delete, `shallowEqual` saw no change and skipped dispatch. LTR now re-resolves the cascade with the would-be inline state and writes an explicit `rightToLeft: false` when the style would still resolve to RTL. Vanilla paragraphs (no style-driven RTL) keep the delete-only behavior so they round-trip clean. 2. `defaultItems.test.js` was checking that the legacy XL_ITEMS list stays in overflow at narrow widths, but did not extend the list to the new `directionLtr` / `directionRtl` items even though they were added to `itemsToHideXL`. The test silently underverified the new items; now exercises them too. 3. Active-state highlight was never applied to items in the overflow popup (pre-existing bug in `super-toolbar.js`, surfaced by this feature because RTL/LTR's highlight is the only signal the buttons give). The active-state loop now iterates `toolbarItems` AND `overflowItems`. * fix(super-editor): direction-ltr/direction-rtl payload type to `never` The headless toolbar payload types for `direction-ltr` and `direction-rtl` declared a `{ direction, alignmentPolicy? }` shape, but `createParagraphDirectionExecute` bakes both fields in (direction from the closure, alignmentPolicy hardcoded to `'matchDirection'`) and ignores any payload. A TS consumer doing `controller.execute('direction-ltr', { direction: 'rtl' })` got LTR regardless of payload. Set both to `never` so the type matches the runtime: this command takes no payload. * test(super-editor): pin direction-ltr/direction-rtl execute contract Adds 4 unit tests proving the runtime contract that justifies the `never` payload type: 1. controller.execute('direction-ltr') -> setParagraphDirection( { direction: 'ltr', alignmentPolicy: 'matchDirection' }) 2. controller.execute('direction-rtl') -> setParagraphDirection( { direction: 'rtl', alignmentPolicy: 'matchDirection' }) 3. payload is ignored if passed (direction comes from the command id) 4. execute returns false when the editor command is unavailable These pin the property that makes ToolbarPayloadMap['direction-*'] = never a faithful declaration of the runtime, not a workaround. * test(super-editor): comprehensive coverage for direction buttons Adds regression coverage across the toolbar click path so the next time someone touches `useToolbarItem({argument})` forwarding, the direction buttons' state, or the `setParagraphDirection` payload shape, tests fail loudly instead of silently breaking the buttons. Unit tests (13 new): - defaultItems.test.js: pin directionLtr/directionRtl item config (command, argument, aria-label) so config drift fails fast. - ButtonGroup.test.js: pin item.argument.value forwarding through the click handler. Plain button click emits the static argument; explicit caller arg wins; disabled item skips emission. Catches the opus review concern about the new fallback affecting all buttons. - toolbar-registry.test.ts: state-deriver tests for direction-ltr / direction-rtl across all three paragraphProperties.rightToLeft states (true / false / undefined). Pins active/value derivation. Playwright behavior tests (5 new) in tests/behavior/tests/toolbar/ paragraph-direction.spec.ts: click the actual button users see, read the ProseMirror doc, assert attrs change. Covers: - RTL click sets paragraphProperties.rightToLeft = true - LTR click on vanilla paragraph deletes the rightToLeft key - RTL → LTR is one undo step (atomic transaction) - Multi-paragraph selection applies to every paragraph - Direction buttons reachable in overflow at narrow viewports All 12751 super-editor unit tests pass. * test(behavior): make paragraph-direction spec use correct fixture API Two real bugs in the spec, not workarounds: - Pin viewport to 1600x1200 in test.use. The Playwright config spreads `devices['Desktop Chrome']` (viewport 1280x720) which silently overrode the global 1600 default, putting the direction buttons in overflow. Below the XL cutoff (~1494px container width), the spec couldn't click the buttons directly. Wider viewport keeps them in the main toolbar. - Fix multi-paragraph selection: setTextSelection takes positional (from, to) args, not an object; findTextPos returns a single number (the start position), not {from,to}. Earlier draft passed an object to setTextSelection so the selection collapsed to undefined/object and only the cursor's paragraph got RTL. The narrow-viewport test deliberately resizes below XL and opens the overflow popup explicitly, so the direct-click helper stays clean. All 15 tests pass across chromium/firefox/webkit. * test(behavior): expand paragraph-direction spec - alignment + active state Adds two end-to-end coverage areas that the unit tests can't reach, and fixes the undo-step test name to match what it actually verifies. - alignmentPolicy plumbing: paragraph has explicit `justification: 'left'`, user clicks Right-to-left button, assert paragraph ends up with `rightToLeft: true` AND `justification: 'right'`. Proves the UI -> command path forwards `alignmentPolicy: 'matchDirection'`, not just the unit-level command logic. - overflow active-state regression for 151cff8: at narrow viewport, click RTL via overflow popup, close, reopen, assert the RTL overflow button has the .active class (and LTR doesn't). Pre-fix this would silently miss because the active-state loop only iterated toolbarItems, not overflowItems. - Rename "Right-to-left then Left-to-right is one undo step" to "Right-to-left click is one undo step" - the test only verified the RTL single-click case. A separate "LTR after RTL is its own undo step" was attempted but PM history coalesces rapid clicks into one step, so that's testing PM behavior, not the PR's contract. All 21 tests pass across chromium/firefox/webkit. * refactor(super-editor): remove direction buttons from default toolbar Per review: directionLtr / directionRtl shouldn't ship in the default toolbar config - they crowd the toolbar for every consumer (including the majority who never touch RTL), and the XL_OVERFLOW_SAFETY_BUFFER had to bump from 20 -> 84 specifically to accommodate them. This commit: - Removes the directionLtr / directionRtl `useToolbarItem` blocks and drops them from `toolbarItems` and `itemsToHideXL`. - Reverts XL_OVERFLOW_SAFETY_BUFFER to 20 (was 84 with a 32px-each-for- direction comment). - Updates defaultItems.test.js: XL_ITEMS no longer includes direction; the 3 direction-config tests are replaced with 2 "not in default toolbar" guard tests so a future re-add will fail loudly. - Deletes tests/behavior/tests/toolbar/paragraph-direction.spec.ts (assumed buttons were in the default toolbar). What stays: - `setParagraphDirection` command + its unit tests - Headless toolbar `direction-ltr` / `direction-rtl` ids + tests (typed payload still `never`; execute/state tests still pin contract) - Icons (`paragraph-ltr-solid.svg`, `paragraph-rtl-solid.svg`) - ButtonGroup argument forwarding tests (generic, not direction-specific) Customer-side: wire the buttons into your own toolbar via the headless toolbar API, or call `editor.commands.setParagraphDirection({ direction })` directly. * test(super-editor): reword ButtonGroup forwarding comment Direction buttons are no longer default toolbar items (see 32153f8), so the test comment shouldn't reference them as the example consumer of this forwarding behavior. Rewords to "custom buttons" since the test itself uses a direction-shaped payload only as an illustrative example. --------- Co-authored-by: Caio Pizzol <97641911+caio-pizzol@users.noreply.github.com> Co-authored-by: Caio Pizzol <caio@superdoc.dev>
1 parent c446999 commit fd1a740

18 files changed

Lines changed: 676 additions & 2 deletions

packages/super-editor/src/editors/v1/components/toolbar/ButtonGroup.test.js

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,3 +56,101 @@ describe('ButtonGroup dropdownOptions selected class', () => {
5656
expect(options[1].props.class).toBe('selected');
5757
});
5858
});
59+
60+
// PR #3226: ButtonGroup forwards a button item's static `argument` (set via
61+
// useToolbarItem({argument})) on click when no caller arg is passed. This is
62+
// how custom buttons carry fixed args like {direction, alignmentPolicy} into
63+
// emit('command'). If this breaks, such buttons become silent no-ops.
64+
describe('ButtonGroup button argument forwarding', () => {
65+
// `type` and `command` are plain (not refs) in useToolbarItem; the rest are refs.
66+
const createButtonItem = (argument) => ({
67+
type: 'button',
68+
command: 'setParagraphDirection',
69+
id: ref('btn-test'),
70+
name: ref('directionLtr'),
71+
argument: argument === undefined ? undefined : ref(argument),
72+
disabled: ref(false),
73+
isNarrow: ref(false),
74+
isWide: ref(false),
75+
tooltip: ref('Test'),
76+
icon: ref(null),
77+
active: ref(false),
78+
expand: ref(false),
79+
attributes: ref({ ariaLabel: 'Test button' }),
80+
});
81+
82+
// shallowMount stubs all children including SdTooltip; SdTooltip is what
83+
// wraps the button branch via <template #trigger>. Provide a custom stub
84+
// that renders its trigger slot so the ToolbarButton stub becomes findable.
85+
const mountButtonItem = (item) =>
86+
shallowMount(ButtonGroup, {
87+
props: { toolbarItems: [item], overflowItems: [] },
88+
global: {
89+
stubs: {
90+
SdTooltip: {
91+
name: 'SdTooltip',
92+
template: '<div><slot name="trigger" /></div>',
93+
},
94+
},
95+
},
96+
});
97+
98+
const findToolbarButton = (wrapper) => wrapper.findComponent({ name: 'ToolbarButton' });
99+
100+
it('plain button click forwards item.argument.value into command emission', () => {
101+
const argument = { direction: 'ltr', alignmentPolicy: 'matchDirection' };
102+
const wrapper = mountButtonItem(createButtonItem(argument));
103+
const button = findToolbarButton(wrapper);
104+
105+
button.vm.$emit('buttonClick');
106+
107+
const events = wrapper.emitted('command');
108+
expect(events).toHaveLength(1);
109+
expect(events[0][0].argument).toEqual(argument);
110+
});
111+
112+
it('emits null argument when item has no static argument', () => {
113+
const wrapper = mountButtonItem(createButtonItem(undefined));
114+
const button = findToolbarButton(wrapper);
115+
116+
button.vm.$emit('buttonClick');
117+
118+
const events = wrapper.emitted('command');
119+
expect(events).toHaveLength(1);
120+
expect(events[0][0].argument).toBeNull();
121+
});
122+
123+
it('directionLtr-shaped item forwards {direction:ltr, alignmentPolicy:matchDirection}', () => {
124+
const argument = { direction: 'ltr', alignmentPolicy: 'matchDirection' };
125+
const wrapper = mountButtonItem(createButtonItem(argument));
126+
const button = findToolbarButton(wrapper);
127+
128+
button.vm.$emit('buttonClick');
129+
130+
const events = wrapper.emitted('command');
131+
expect(events[0][0].argument.direction).toBe('ltr');
132+
expect(events[0][0].argument.alignmentPolicy).toBe('matchDirection');
133+
});
134+
135+
it('directionRtl-shaped item forwards {direction:rtl, alignmentPolicy:matchDirection}', () => {
136+
const argument = { direction: 'rtl', alignmentPolicy: 'matchDirection' };
137+
const wrapper = mountButtonItem(createButtonItem(argument));
138+
const button = findToolbarButton(wrapper);
139+
140+
button.vm.$emit('buttonClick');
141+
142+
const events = wrapper.emitted('command');
143+
expect(events[0][0].argument.direction).toBe('rtl');
144+
expect(events[0][0].argument.alignmentPolicy).toBe('matchDirection');
145+
});
146+
147+
it('skips command emission when item is disabled', () => {
148+
const disabledItem = { ...createButtonItem({ direction: 'ltr' }), disabled: ref(true) };
149+
const wrapper = mountButtonItem(disabledItem);
150+
const button = findToolbarButton(wrapper);
151+
152+
button.vm.$emit('buttonClick');
153+
154+
expect(wrapper.emitted('command')).toBeUndefined();
155+
});
156+
});

packages/super-editor/src/editors/v1/components/toolbar/ButtonGroup.vue

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,11 @@ const handleToolbarButtonClick = (item, argument = null) => {
114114
}
115115
116116
emit('item-clicked');
117-
emit('command', { item, argument });
117+
// Forward the item's static `argument` (set via `useToolbarItem({ argument })`)
118+
// when no caller-provided argument exists. Lets buttons carry fixed args like
119+
// `{ direction: 'rtl' }` without needing a dropdown.
120+
const resolved = argument ?? item.argument?.value ?? null;
121+
emit('command', { item, argument: resolved });
118122
};
119123
120124
const handleToolbarButtonTextSubmit = (item, argument) => {

packages/super-editor/src/editors/v1/components/toolbar/constants.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ export const HEADLESS_ITEM_MAP = {
8888
linkedStyles: 'linked-style',
8989
indentleft: 'indent-decrease',
9090
indentright: 'indent-increase',
91+
directionLtr: 'direction-ltr',
92+
directionRtl: 'direction-rtl',
9193
clearFormatting: 'clear-formatting',
9294
copyFormat: 'copy-format',
9395
};

packages/super-editor/src/editors/v1/components/toolbar/defaultItems.test.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,3 +95,25 @@ describe('makeDefaultItems LG compact styles', () => {
9595
expect(linkedStyles.attributes.value.className).not.toContain('toolbar-item--linked-styles-compact');
9696
});
9797
});
98+
99+
// PR #3226: direction buttons (directionLtr / directionRtl) are intentionally
100+
// NOT in the default toolbar. The command (`setParagraphDirection`) and the
101+
// headless toolbar ids (`direction-ltr` / `direction-rtl`) stay available;
102+
// customers wire them into their own UI via the headless toolbar API or by
103+
// calling the command directly. Pin "not in default" here so a future
104+
// re-add in makeDefaultItems fails this test instead of silently shipping.
105+
describe('makeDefaultItems direction buttons not in default toolbar', () => {
106+
function getItem(defaultItems, overflowItems, name) {
107+
return [...defaultItems, ...overflowItems].find((item) => item.name.value === name);
108+
}
109+
110+
it('directionLtr is not in the default toolbar items', () => {
111+
const { defaultItems, overflowItems } = buildItems(2000);
112+
expect(getItem(defaultItems, overflowItems, 'directionLtr')).toBeUndefined();
113+
});
114+
115+
it('directionRtl is not in the default toolbar items', () => {
116+
const { defaultItems, overflowItems } = buildItems(2000);
117+
expect(getItem(defaultItems, overflowItems, 'directionRtl')).toBeUndefined();
118+
});
119+
});

packages/super-editor/src/editors/v1/components/toolbar/super-toolbar.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,11 @@ export class SuperToolbar extends EventEmitter {
712712
return;
713713
}
714714

715-
this.toolbarItems.forEach((item) => {
715+
// Overflow items still appear in the overflow popup and need their
716+
// active-state highlight (e.g., bold pressed, direction matched) to
717+
// reflect the current selection. Iterating only `toolbarItems` left
718+
// them frozen in their last-rendered state.
719+
[...this.toolbarItems, ...this.overflowItems].forEach((item) => {
716720
item.resetDisabled();
717721
this.#applyHeadlessState(item);
718722
});

packages/super-editor/src/editors/v1/components/toolbar/toolbarIcons.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ import copyIconSvg from '@superdoc/common/icons/copy-solid.svg?raw';
6060
import pasteIconSvg from '@superdoc/common/icons/paste-solid.svg?raw';
6161
import strikethroughSvg from '@superdoc/common/icons/strikethrough.svg?raw';
6262
import paragraphIconSvg from '@superdoc/common/icons/paragraph-solid.svg?raw';
63+
import paragraphLtrIconSvg from '@superdoc/common/icons/paragraph-ltr-solid.svg?raw';
64+
import paragraphRtlIconSvg from '@superdoc/common/icons/paragraph-rtl-solid.svg?raw';
6365

6466
export const toolbarIcons = {
6567
undo: rotateLeftIconSvg,
@@ -89,6 +91,8 @@ export const toolbarIcons = {
8991
numberedListLowerAlphaParen: listLowerAlphaParenIconSvg,
9092
indentLeft: outdentIconSvg,
9193
indentRight: indentIconSvg,
94+
directionLtr: paragraphLtrIconSvg,
95+
directionRtl: paragraphRtlIconSvg,
9296
pageBreak: fileHalfDashedIconSvg,
9397
copyFormat: paintRollerIconSvg,
9498
clearFormatting: textSlashIconSvg,

packages/super-editor/src/editors/v1/components/toolbar/toolbarTexts.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ export const toolbarTexts = {
2929
numberedList: 'Numbered list',
3030
indentLeft: 'Left indent',
3131
indentRight: 'Right indent',
32+
directionLtr: 'Left-to-right',
33+
directionRtl: 'Right-to-left',
3234
zoom: 'Zoom',
3335
undo: 'Undo',
3436
redo: 'Redo',

packages/super-editor/src/editors/v1/core/commands/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export * from './insertSectionBreakAtSelection.js';
4444
// Paragraph
4545
export * from './textIndent.js';
4646
export * from './lineHeight.js';
47+
export * from './paragraphDirection.js';
4748

4849
// Run
4950
export * from './backspaceEmptyRunParagraph.js';
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import { resolveHypotheticalParagraphProperties } from '@extensions/paragraph/resolvedPropertiesCache.js';
2+
3+
/**
4+
* Set paragraph direction (LTR/RTL) on every paragraph in the current selection.
5+
* @category Command
6+
* @param {Object} input
7+
* @param {"ltr"|"rtl"} input.direction
8+
* @param {"matchDirection"} [input.alignmentPolicy] - When set to "matchDirection",
9+
* mirror an *explicit* `justification` of "left" ↔ "right" so the alignment
10+
* follows the new direction. Leaves "center", "both", and unset values alone.
11+
* @returns {Function} ProseMirror command function
12+
* @example
13+
* editor.commands.setParagraphDirection({ direction: 'rtl', alignmentPolicy: 'matchDirection' })
14+
*/
15+
export const setParagraphDirection = ({ direction, alignmentPolicy } = {}) => {
16+
// Guard against headless callers that invoke this through a generic
17+
// "execute by command name" pathway without a payload — a missing
18+
// direction must be a no-op, not a silent LTR write.
19+
if (direction !== 'ltr' && direction !== 'rtl') return () => false;
20+
return walkParagraphs((pPr, { editor, $pos }) => {
21+
const next = { ...pPr };
22+
if (direction === 'rtl') {
23+
next.rightToLeft = true;
24+
} else {
25+
// AIDEV-NOTE: LTR first tries to delete the inline override (so a
26+
// vanilla paragraph round-trips without `<w:bidi w:val="0"/>`). But
27+
// if the paragraph inherits `rightToLeft: true` from its style (or
28+
// any other level of the OOXML cascade), deleting alone leaves the
29+
// resolved direction as RTL — clicking LTR would be a silent no-op.
30+
// Re-resolve the cascade against the would-be inline state; if RTL
31+
// still wins, force an explicit `false` to override the style.
32+
delete next.rightToLeft;
33+
const resolved = resolveHypotheticalParagraphProperties(editor, $pos, next);
34+
if (resolved?.rightToLeft === true) {
35+
next.rightToLeft = false;
36+
}
37+
}
38+
if (alignmentPolicy === 'matchDirection') {
39+
const j = pPr.justification;
40+
if (j === 'left' && direction === 'rtl') next.justification = 'right';
41+
else if (j === 'right' && direction === 'ltr') next.justification = 'left';
42+
}
43+
return next;
44+
});
45+
};
46+
47+
/**
48+
* Clear an explicit paragraph direction override on every paragraph in the
49+
* current selection. The paragraph reverts to its auto-resolved direction.
50+
* @category Command
51+
* @returns {Function} ProseMirror command function
52+
* @example
53+
* editor.commands.clearParagraphDirection()
54+
*/
55+
export const clearParagraphDirection = () =>
56+
walkParagraphs((pPr) => {
57+
const next = { ...pPr };
58+
delete next.rightToLeft;
59+
return next;
60+
});
61+
62+
function walkParagraphs(transform) {
63+
return ({ editor, state, dispatch }) => {
64+
const { from, to } = state.selection;
65+
const tr = state.tr;
66+
let touched = false;
67+
68+
state.doc.nodesBetween(from, to, (node, pos) => {
69+
if (node.type.name !== 'paragraph') return true;
70+
71+
const existing = node.attrs.paragraphProperties || {};
72+
const updated = transform(existing, { editor, node, $pos: state.doc.resolve(pos) });
73+
74+
if (shallowEqual(existing, updated)) return false;
75+
76+
tr.setNodeMarkup(pos, undefined, {
77+
...node.attrs,
78+
paragraphProperties: updated,
79+
});
80+
touched = true;
81+
return false;
82+
});
83+
84+
if (touched && dispatch) dispatch(tr);
85+
return touched;
86+
};
87+
}
88+
89+
function shallowEqual(a, b) {
90+
const ka = Object.keys(a);
91+
const kb = Object.keys(b);
92+
if (ka.length !== kb.length) return false;
93+
for (const k of ka) {
94+
if (a[k] !== b[k]) return false;
95+
}
96+
return true;
97+
}

0 commit comments

Comments
 (0)