feat(super-editor): native LTR/RTL paragraph direction toolbar#3226
feat(super-editor): native LTR/RTL paragraph direction toolbar#3226shri-scale wants to merge 5 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c445e4912a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@caio-pizzol Kindly have a look |
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 superdoc-dev#3219.
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.
…load 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.
…riting 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.
shri-scale
force-pushed
the
paragraph-direction-toolbar
branch
from
534dfc3 to
3542424
Compare
There was a problem hiding this comment.
hey @shri-scale! thanks for the PR - delete-on-ltr matching what Word actually writes is a nice touch :)
Important
needs work, three things to address.
users in an rtl paragraph see neither LTR nor RTL highlight in the overflow popup, and clicking LTR on a paragraph that inherits rtl from a style does nothing. one is out of diff (super-toolbar.js:715); two more inline.
| // vanilla paragraph indistinguishable from one that's been toggled | ||
| // RTL → LTR. | ||
| if (direction === 'rtl') next.rightToLeft = true; | ||
| else delete next.rightToLeft; |
There was a problem hiding this comment.
important: clicking LTR on a paragraph whose style sets rightToLeft: true is a silent no-op.
verified by tracing the cascade at style-engine/src/ooxml/index.ts:154 resolveParagraphProperties -> deleting rightToLeft from inline props lets styleProps.rightToLeft win. fix not verified.
likely fix: write rightToLeft: false only when the resolved direction would still be rtl after the delete. keeps the current behavior for vanilla paragraphs and forces an explicit override only where the style cascade would otherwise win.
| const XL_OVERFLOW_SAFETY_BUFFER = 20; | ||
| const XL_OVERFLOW_SAFETY_BUFFER = 84; | ||
| const XL_CUTOFF = RESPONSIVE_BREAKPOINTS.xl + XL_OVERFLOW_SAFETY_BUFFER; | ||
| const XL_ITEMS = ['linkedStyles', 'clearFormatting', 'copyFormat', 'ruler']; |
There was a problem hiding this comment.
nit: defaultItems.js:1097-1108 now hides directionLtr / directionRtl at XL, but XL_ITEMS here never picked them up.
| const XL_ITEMS = ['linkedStyles', 'clearFormatting', 'copyFormat', 'ruler']; | |
| const XL_ITEMS = ['linkedStyles', 'clearFormatting', 'copyFormat', 'ruler', 'directionLtr', 'directionRtl']; |
2 participants
Summary
<w:bidi>onpPr.rightToLeftand exposes a Document API mutation, but the toolbar didn't surface it. Users editing Arabic/Hebrew docs were patching this gap with custom buttons.What changed
setParagraphDirection/clearParagraphDirectionProseMirror commands (packages/super-editor/src/editors/v1/core/commands/paragraphDirection.js). Walk paragraphs in the current selection, togglepPr.rightToLeft, optionally mirror an explicitjustificationof"left"↔"right"whenalignmentPolicy: "matchDirection"is passed (leaves"center","both", and unset alone). Multi-paragraph selection produces a single transaction → one undo step.Two default toolbar buttons next to
indentLeft/indentRight(directionLtr,directionRtl) with Material Designformat_textdirection_*icons. Active state is wired through the headless toolbar registry (createParagraphDirectionStateDeriverreadspPr.rightToLeft), so the matching button highlights as the cursor moves between LTR and RTL paragraphs.ButtonGroup.vuepatch. Plain button clicks previously hardcodedargument: null. They now forwarditem.argument?.valueso any built-in button can carry a static argument (the direction buttons need{ direction, alignmentPolicy }). Generic fix; doesn't affect existing buttons.XL overflow buffer bumped from 20 → 84 to accommodate the two extra items at the existing XL cutoff.
Test plan
pnpm --filter @superdoc/super-editor test— new vitest covers RTL/LTR set,matchDirectionflip behavior across all justification values, single-transaction multi-paragraph, andclearParagraphDirectionsemantics<w:bidi/>survives