feat: add per-block RTL support - #277
Conversation
|
@n00ki is attempting to deploy a commit to the bholmesdev's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Overview
Per-block RTL via TipTap textDirection: "auto", LTR code/source, and CSS so list/quote chrome follows block direction. Empty new blocks try to keep the previous side until the first strong character.
Approach is sound (native TipTap, no Markdown metadata). One correctness gap breaks the empty-block inheritance story after the first blank line.
Concerns
- Empty-block CSS keys off
:dir(ltr|rtl)on the previous sibling. Empty nodes keep HTMLdir="auto", so they never match:dir(ltr|rtl)even after CSS setsdirection. Second+ blank line after RTL loses the intended side (double Enter, blank between paragraphs). - Tests only assert
dirattributes, not resolved direction or empty-block inheritance. - No
CHANGELOG.md[Unreleased]entry for this user-facing change.
Verdict
Found: 0 critical, 1 important, 2 suggestions
Request changes
Reviewed by a Warp Factory agent.
| [data-hubble-editor] | ||
| .ProseMirror | ||
| :is( | ||
| :dir(rtl) + p:has(> br.ProseMirror-trailingBreak:only-child), |
There was a problem hiding this comment.
:dir(ltr|rtl) + empty p only matches when the previous element’s HTML direction is ltr/rtl. TipTap keeps empty blocks at dir="auto", and CSS direction does not change :dir(). After one blank line inherits RTL from a strong block, the next blank’s previous sibling is still dir=auto, so it drops out of these rules and the caret jumps LTR (double Enter / several empty lines).
Same issue on the LTR twin above. Fix: also match a previous empty that already got temporary direction (extra selector / data attr), or set real dir on empty blocks in JS from the previous strong block until content arrives.
| const editor = getEditor(); | ||
| expect(editor.getAttribute("dir")).toBe("auto"); | ||
| for (const selector of ["h1", "p", "blockquote", "ul", "li"]) { | ||
| const blocks = editor.querySelectorAll(selector); | ||
| expect(blocks.length).toBeGreaterThan(0); | ||
| for (const block of blocks) | ||
| expect(block.getAttribute("dir")).toBe("auto"); | ||
| } | ||
| expect(editor.querySelector(".pm-code-block")?.getAttribute("dir")).toBe( | ||
| "ltr", | ||
| ); |
There was a problem hiding this comment.
💡 [SUGGESTION] These asserts only check dir="auto"|"ltr" attributes, which TipTap sets globally. They miss the CSS empty-block inheritance path and never assert computed/used direction for Hebrew/Arabic content or consecutive empty paragraphs. Prefer at least one case that documents the double-Enter caret side (and would have caught the :dir() issue above).
|
|
||
| const editor = useEditor({ | ||
| editable, | ||
| textDirection: "auto", |
There was a problem hiding this comment.
💡 [SUGGESTION] User-facing RTL support needs a CHANGELOG.md bullet under ## [Unreleased] → ### Added (CONTRIBUTING). One line on auto per-block direction for mixed LTR/RTL notes.
Rationale
Hubble lays out editor content from left to right, which makes Hebrew, Arabic, and other RTL text awkward to write and read.
This adds automatic direction detection per block, allowing LTR and RTL content to coexist in the same document. New empty blocks initially follow the preceding block’s direction, preventing the caret from jumping when writing several lines in the same language.
Alternatives considered
Summary
Testing
pnpm --filter @hubble.md/ui testpnpm checkpnpm check:react-compilerpnpm build:desktop