Conversation
- Detect text direction from the DOM and adjust cursor caret position for RTL text (Hebrew, Arabic, etc.) - Quill's `getBounds()` returns `width: 0` for cursor positions, so RTL caret position is computed directly from character bounding rects via `document.createRange()` - Direction detection uses fast `dir` attribute lookup, falling back to `getComputedStyle` for CSS-only direction - No breaking changes — fully backward compatible
fyvfyv
force-pushed
the
rtl-cursor-positioning
branch
from
3d8db9f to
23f9a44
Compare
|
@alecgibson Could you please review this PR when you get a chance? Also, the ESLint config used in this repo relies on a private Thanks! |
src/quill-cursors/quill-cursors.ts
Outdated
| const dirEl = element.closest('[dir]'); | ||
| if (dirEl) return (dirEl as HTMLElement).dir === 'rtl'; |
There was a problem hiding this comment.
Shouldn't this be covered by getComputedStyle()? Is it here for performance? If so, I think I'd rather we start with one "canonical" way of figuring out the direction with getComputedStyle() and then see if this becomes an issue. Also, direction can be set through CSS, so you can in theory have this sort of thing:
<div dir="rtl">
<div style="direction: ltr;"></div>
</div>And this current code would incorrectly say that we're in an RTL environment when we're actually not.
There was a problem hiding this comment.
Good catch You're right - the closest('[dir]') shortcut is both redundant and buggy. Simplified _isRtl to only use getComputedStyle(element).direction.
src/quill-cursors/quill-cursors.ts
Outdated
| const range = document.createRange(); | ||
| if (offset < node.data.length) { | ||
| range.setStart(node, offset); | ||
| range.setEnd(node, offset + 1); | ||
| } else { | ||
| range.setStart(node, offset - 1); | ||
| range.setEnd(node, offset); | ||
| } | ||
|
|
||
| const charRect = range.getBoundingClientRect(); |
There was a problem hiding this comment.
Took me a minute to figure out what we were doing here (sorry I haven't thought hard about RTL before). Might be helpful to move this to a function named something (long and) obvious like getWidthOfCharacterToRightOfCursor()
There was a problem hiding this comment.
Agreed, extracted it into _getCharacterRectAtCursor(node, offset). It returns a DOMRect for the character adjacent to the cursor offset. Open to a different name if you have a preference.
2 participants
getBounds()returnswidth: 0for cursor positions, so RTL caret position is computed directly from character bounding rects viadocument.createRange()dirattribute lookup, falling back togetComputedStylefor CSS-only directionFix #86