fix: Make RTL work in dir="rtl" html #60
Conversation
rijkvanzanten
changed the title
There was a problem hiding this comment.
Pull request overview
This PR fixes RTL (right-to-left) support in the split panel component to work correctly when the HTML document has dir="rtl". The previous implementation incorrectly reversed the grid template order for RTL layouts, which caused issues when the browser itself was in RTL mode. The fix relocates RTL handling from the grid template layer to the pointer/drag handling layer, allowing CSS Grid's native RTL support to handle layout direction.
Key Changes:
- Moved RTL position transformation from snap point iteration to initial drag position calculation
- Removed direction-based grid template reordering, relying on CSS Grid's native RTL support
- Updated tests to reflect the corrected RTL behavior
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/vue-split-panel/src/composables/use-pointer.ts | Added RTL transformation to drag position (lines 34-36), removed problematic snap point transformation, changed loop variable to const |
| packages/vue-split-panel/src/composables/use-pointer.test.ts | Updated RTL test expectations to match new behavior where drag position is mirrored rather than snap points |
| packages/vue-split-panel/src/composables/use-grid-template.ts | Removed Direction import and direction parameter, simplified grid template logic to use CSS Grid's native RTL support |
| packages/vue-split-panel/src/composables/use-grid-template.test.ts | Removed direction parameter from test setup and deleted RTL-specific test for old (buggy) behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // There's a tsdown/vitest issue causing false positives here, so we skip these tests for now | ||
| // => TypeError: Cannot define property split-panel, object is not extensible |
There was a problem hiding this comment.
These tests are being skipped due to a tsdown/vitest issue. Consider creating a tracking issue or TODO comment with a link to the upstream issue so this can be re-enabled once the issue is resolved. Skipping tests reduces code coverage and may hide regressions.
| // There's a tsdown/vitest issue causing false positives here, so we skip these tests for now | ||
| // => TypeError: Cannot define property split-panel, object is not extensible |
There was a problem hiding this comment.
These tests are being skipped due to a tsdown/vitest issue. Consider creating a tracking issue or TODO comment with a link to the upstream issue so this can be re-enabled once the issue is resolved. Skipping tests reduces code coverage and may hide regressions.
The
direction="rtl"prop worked great inltrcontexts, but would fail when the direction isrtlin the browser itself.