Skip to content

feat: add core RTL support for headers and column resizing - #1271

Open
jahanbakhsh18 wants to merge 3 commits into
6pac:masterfrom
jahanbakhsh18:feature/rtl-support
Open

feat: add core RTL support for headers and column resizing#1271
jahanbakhsh18 wants to merge 3 commits into
6pac:masterfrom
jahanbakhsh18:feature/rtl-support

Conversation

@jahanbakhsh18

@jahanbakhsh18 jahanbakhsh18 commented Aug 7, 2026

Copy link
Copy Markdown

fixes #446

This PR adds foundational RTL (Right-to-Left) support to SlickGrid, focusing on:

  • RTL header alignment
  • Column resizing in RTL mode
  • CSS property swapping for RTL

Changes

  • Added dirSide getter for dynamic CSS property selection
  • Updated header container positioning to use [dirSide]
  • Modified applyColumnWidths to swap left/right for RTL
  • Updated column resize logic for RTL
  • Added example1-simple-rtl.html

Testing

Manual testing confirmed:

  • Headers display in correct RTL order
  • Resize handles appear on the left side
  • Column resizing works correctly
  • No regression in LTR mode

Breaking Changes

None. rtl option defaults to false.

Cypress Test Result: https://github.com/user-attachments/files/30840967/test-results-chrome.txt
Demo: https://github.com/user-attachments/assets/903c56a2-8472-43ed-8615-e567146685a2

Comment thread examples/example1-simple-rtl.html Outdated
@ghiscoding

ghiscoding commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Can you please provide a print screen of the result, so we can visually see the result without having to run it locally.
Also you should probably use some of the variable that I added in the default theme to change the cells to RTL as well. The last time we discussed RTL was in #446 which you may want to read for reference (side note, I have never used RTL anywhere, so knowledge is very basic)

@6pac the PR seems ok but asking Claude to double-check might be a good idea.

$alpine-cell-justify-content: flex-start !default;

$alpine-header-align-items: flex-start !default;
$alpine-header-justify-content: flex-start !default;

Any of the SASS variables can be used by simply replacing the $ symbol to --, for example:

body {
  --alpine-cell-justify-content: flex-end;
}

Comment thread src/slick.grid.ts Outdated
@jahanbakhsh18

jahanbakhsh18 commented Aug 7, 2026

Copy link
Copy Markdown
Author

Thank you for the feedback! Let me clarify the approach and scope of this PR:

Core Changes That CSS Cannot Handle
The key contributions in this PR are the logic changes in slick.grid.ts that CSS alone cannot achieve:

  1. Resize direction reversal in RTL
  2. RTL-aware resize constraints
    These changes handle the resize handle behavior in RTL mode, which is fundamentally a logic problem, not a styling one.

Note: The _headerL, _headerR, createCssRules(), and applyColumnWidths() are all bound to the code. While CSS variables like --alpine-cell-justify-content: flex-end would help with cell content alignment.

Refactoring Opportunity
I agree that some parts could be refactored to be more CSS-friendly. For example, the _headerL and _headerR positioning could potentially use CSS variables. However, the resize logic in applyColumnWidths() and the drag constraints require code-level changes regardless of how we refactor the CSS.

Screenshot from 2026-08-08 00-20-15

@jahanbakhsh18

jahanbakhsh18 commented Aug 8, 2026

Copy link
Copy Markdown
Author

Regarding dirSide()
You're right that there are ways to remove dirSide() and push more to CSS. However, any approach will still require changes in slick.grid.ts because:

  • The resize logic needs RTL-aware calculations (the d = -d and constraint changes)
  • The header positioning needs to be applied to DOM elements
  • The new .slick-rtl class needs to be added programmatically based on the rtl option

CSS alone cannot handle these behavioral changes. But I'm happy to explore removing dirSide() if you prefer a different approach. If you'd like me to remove dirSide() and use a different approach, keep it as-is for now, or explore alternative implementations. This PR is the foundation for RTL support, but there's more work ahead. I'm committed to seeing this through and helping with future improvements:

  • Column reordering in RTL which requires discussion around using sortable-rtl or post-sortable architecture (drop SortableJS)
  • Scroller behavior in RTL
  • Comprehensive E2E tests for RTL scenarios
  • Any other issues that come up

Let me know your thoughts on the path forward...

@6pac

6pac commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Here is Claude Code's review (quite a lot!). Let me know if you want a suggested work plan from it.


PR #1271 — core RTL support for headers and column resizing

Summary

The direction is welcome and the approach is refreshingly minimal: an opt-in flag that defaults to false, a small diff, and no restructuring of existing code. The header alignment and column-resize work does what it claims.

However, "core RTL support" currently overstates what works. Two supported code paths are broken once an RTL grid is wide enough to scroll horizontally, and both are invisible in the accompanying demo because the demo cannot scroll horizontally at all. The gap is not in what the PR changed — it is in the surface it did not reach.

How this was verified

Rather than reading the diff alone, the branch was checked out, built (tsc and eslint clean, prod build exit 0), and driven in a browser. Each finding below is a reproduction with measured numbers, and the key one is paired with an LTR control run so it is clear the behaviour is RTL-specific rather than a pre-existing quirk or a flaw in the test method.

The full Cypress suite was not run on this branch, so nothing here should be read as a broad regression claim — CI covers that.


Blocking issues

1. scrollCellIntoView() is a no-op in RTL

This is the most significant finding, because it is a public API silently doing nothing.

Identical grids, identical column widths (6 columns × 200px in a 583px viewport), identical call — grid.scrollCellIntoView(0, 5), asking to bring the last column into view:

scrollLeft after the call columns on screen
LTR (example1-simple) 617 (scrolled) 3, 4, 5 — target visible
RTL (example1-simple-rtl) 0 (no movement) 0, 1, 2 — target off-screen

The cause is the sign convention. In RTL, element.scrollLeft starts at 0 at the right edge and runs negative down to -(scrollWidth - clientWidth). internalScrollColumnIntoView — along with the rest of the horizontal machinery — assumes the LTR convention of a positive 0…max range, so its "is this column already visible?" test returns true for a column that is in fact off-screen, and no scroll is issued.

This is not an isolated call site. slick.grid.ts contains 51 references to scrollLeft, and this PR changes none of them. The same assumption shows up in cell virtualization: driving the viewport to the far end (scrollLeft = -617) leaves the visible area with zero cells rendered, because the computed leftPx/rightPx window lands in the wrong place and cleanUpAndRenderCells removes precisely the cells that are on screen.

Practically, this means an RTL grid works as long as every column fits, and degrades badly as soon as it does not.

2. Frozen columns overlap in RTL

Applying setOptions({ frozenColumn: 1 }) to the RTL demo produces panes that sit on top of one another:

frozen pane:      left -412, width 400  ->  spans -412 … -12
scrollable pane:  left -210, width 198  ->  spans -210 … -12   (entirely inside the frozen pane)

In LTR these two panes are adjacent and non-overlapping. Pane positioning is done with left offsets that were not mirrored, so in RTL the scrollable pane is drawn over the frozen one instead of beside it. In RTL the frozen pane should be on the right, with the scrollable pane extending to its left.

Fixing frozen panes may reasonably be out of scope for a first RTL PR — but in that case the combination should be explicitly rejected, or documented as unsupported, rather than silently rendering overlapping panes.

3. The demo cannot exercise either problem

example1-simple-rtl.html uses six columns at default widths — 80, 80, 90, 80, 80, 90 — totalling 500px inside a 583px viewport. Measured: scrollWidth === clientWidth, so the grid never scrolls horizontally.

This explains how "manual testing confirmed... no regression" could hold while issues 1 and 2 were present: the demo only exercises the case where everything fits. Widening the demo, or adding a few more columns so it genuinely scrolls, is the single highest-value change to make to this PR — it surfaces issue 1 immediately and makes the feature demonstrable rather than merely plausible.


Design suggestions

4. The !important override belongs in the library, not the example

The example carries this workaround:

.slick-resizable-handle { right: auto !important; left: 0 !important; }

If every consumer has to hand-write !important CSS to get resize handles on the correct side, the library does not really support RTL — it tolerates it. This rule belongs in slick.grid.scss or the themes, scoped behind a slick-rtl class on the container (or [dir="rtl"] selectors), so that setting rtl: true is sufficient on its own.

5. The grid never sets direction on its own container

The demo relies on <html dir="rtl"> for the actual direction; the rtl option only drives the JavaScript-side positioning. That means an RTL grid embedded in an otherwise LTR page will not render correctly, which is the common real-world case — an admin screen in English containing one Arabic or Hebrew data table.

Setting direction: rtl (or an slick-rtl class) on the grid container from the option would make the feature self-contained.

6. CSS logical properties would remove most of the branching

inset-inline-start and inset-inline-end resolve to left/right according to the container's direction. Adopting them instead of left/right would collapse this work into a single code path: no dirSide getter, no if (this._options.rtl) branch in applyColumnWidths, and layout that follows direction automatically without a parallel set of RTL rules to maintain.

The current approach works, but it establishes a pattern where every future left/right site in the codebase needs the same manual mirroring, and each one is a place where LTR and RTL can drift apart. Browser support for logical properties is now universal, so this is worth weighing before the branching approach becomes entrenched.

7. Runtime toggling is only partly handled

createCssRules() is re-run by setColumns, so the .lN/.rN and .slick-header-column rules are correctly regenerated if rtl changes at runtime. But the inline style on the header containers — { [this.dirSide]: '-1000px' } on _headerL and _headerR — is written once during construction and never revisited, so setOptions({ rtl }) would leave a stale left: -1000px behind.

Either update those two elements when the option changes, or document rtl as a construction-time-only option so nobody expects otherwise.

8. Consider auto-detecting direction

getComputedStyle(container).direction === 'rtl' would let the grid pick up the ambient direction, with the explicit rtl option retained as an override. This removes the failure mode where the option and the DOM disagree — currently, setting rtl: true on a page that is not RTL, or vice versa, produces a half-mirrored grid with no warning.


Smaller items

  • Getter type. protected get dirSide() infers string, not 'left' | 'right'. Annotating the return type keeps the computed-key style object ({ [this.dirSide]: '-1000px' }) type-safe.
  • Charset. example1-simple-rtl.html declares charset=iso-8859-1. For an RTL demo this must be UTF-8, or any Arabic, Persian or Hebrew content will mojibake.
  • Description vs. code. The PR description mentions "Persian sample data", but the committed data is English (Task 0, 5 days). It also says "Added hideSide getter", while the code has dirSide.
  • Missing newline at the end of example1-simple-rtl.html.
  • No automated test. The recent convention in this repo (fix: frozen-bottom grids never clean up off-screen row cells #1255, fix: footer-row lifecycle - construction crash, inconsistent getFooterRow, double onFooterRowCellRendered #1256, fix: fractional grid height leaves the bottom rows unrendered (#1262) #1263) is a Cypress spec landing in the same PR as the change; this one links an external test-results.txt instead. A spec asserting header/cell alignment and scrollCellIntoView behaviour under RTL would have caught issue 1, and would keep the feature from regressing as the surrounding code changes.
  • JSDoc scope. The dirSide comment describes only the "hide header columns off-screen" use, but the getter is also used for real header container positioning.

Checked and not an issue

The resize direction logic holds up. The handle sits on the column's visual left edge in RTL, which is still the boundary between columns i and i+1, so swapping the shrink/stretch leeways in the minPageX/maxPageX clamp and then negating d are complementary rather than double-correcting: physical movement is clamped in physical space, then converted to a logical delta.

Header and cell positions stayed in lockstep at every scroll position tested, in both directions — the header/body synchronisation is not affected by the scrollLeft sign problem described in issue 1.


Suggested path forward

  1. Widen the demo so it scrolls horizontally — this is what makes the remaining work visible and reviewable.
  2. Address the scrollLeft sign convention, or scope the feature explicitly to non-scrolling grids for now.
  3. Decide on frozen columns: mirror the panes, or reject the combination with a clear message.
  4. Move the resize-handle CSS into the themes.
  5. Add a Cypress spec covering alignment and scrollCellIntoView in RTL.

Points 1, 4 and 5 are small. Point 2 is the real work, and is worth deciding on before the branching pattern in point 6 above spreads any further.

@jahanbakhsh18

Copy link
Copy Markdown
Author

Thank you for the incredibly thorough review, @6pac. I truly appreciate the time and deep testing you've put into this.

I've read through the feedback and agree with the assessment. The scrollLeft sign convention is clearly the critical piece that needs to be addressed for RTL to work properly in real-world grids.

Let me take some time to process the full review and I'll come back with a detailed work plan. I'll focus first on fixing the scroller issue and will provide clear updates soon.

Thanks again for your time and guidance!

@6pac

6pac commented Aug 10, 2026

Copy link
Copy Markdown
Owner

@jahanbakhsh18 I'd love to take credit, but note that that was entirely done by Claude Code. I'm happy to get it to create work plan if you like, it costs almost nothing and saves us all a huge amount of time.
It can then also create all the commits and the PR. We just need to review what it has done and make sure we're happy with it.

@jahanbakhsh18

Copy link
Copy Markdown
Author

I appreciate the offer! I use AI tools myself and know they can be incredibly helpful for certain tasks.
However, I've invested significant time understanding the slick.grid.ts rendering pipeline and the scrollLeft sign convention problem for this specific RTL issue. I want to see this through myself. I will work on this immediately and intensively until it's ready. Thanks again for your trust and guidance.

@ghiscoding

ghiscoding commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

@jahanbakhsh18 we're about to do a release though, so would it better to just merge what we have now and you can push some fix later? unless you can have it done by tomorrow, which should be about the time we push a release!?

@jahanbakhsh18

jahanbakhsh18 commented Aug 10, 2026

Copy link
Copy Markdown
Author

That's great news. I'd love to have this included in the upcoming release!
I've made huge progress on the scrollLeft fix:

  • Scroller fix: I've identified the sign convention issue and have a working solution
  • Demo: I've updated example1-simple-rtl.html with more columns to force horizontal scrolling
  • Cypress tests: I have new RTL-specific tests, and importantly, the rtl parameter check ensures they don't affect any existing LTR tests. so the test suite remains clean and reliable.

I can definitely have the scroller fix, updated demo, and RTL Cypress tests completed and pushed by the deadline. What time tomorrow do you need the commit by? I'll work backwards from that to make sure everything is ready.
Once I know the cutoff, I'll make sure the PR is fully updated and ready for review in time for the release.
Thank you for including this in the release. I'll make sure it's solid!

@ghiscoding

ghiscoding commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

@jahanbakhsh18 I'll push after my working hours on Monday, around 7-8pm EST (Eastern TimeZone). I'm in Canada and @6pac (Ben) is in Australia, day and night 😆

Side note, I have my own SlickGrid repo and I've replicated your changes in mine too but I had to adjust a few things to get it all working. For example, I'm not sure if you tested with the Grid Menu, Header Menu, Column Picker, ... but in my repo, all these things are enabled by default and I have my own CSS styling themes as well, so a few CSS changes were required, there's a print screen of what it looks like in my repo and I didn't know that RTL really inverses everything, even column picker checkbox right instead of left, it feels a bit weird for us 😄
ghiscoding/slickgrid-universal#2714

@jahanbakhsh18

jahanbakhsh18 commented Aug 10, 2026

Copy link
Copy Markdown
Author

Thank you so much @ghiscoding for the update! 😄 The timezone dance...
I'll make sure to get everything in before your evening push. Wow, I just checked your screenshot. The RTL support looks fantastic!
I actually tested the Grid Menu and attached a screenshot showing it working with minimal changes. You're right that full CSS alignment for all components will take more work, but the core functionality is solid.

Here's what I'm committing today:

  • scrollLeft fix: The getVisibleRange fix that addresses the critical issue for horizontal scrolling (@6pac's review)
  • Cypress tests: Core Cypress RTL tests
  • Expanded demo: example1-simple-rtl.html now has more columns to force horizontal scrolling
  • Minor fixes: From @6pac's review (resize handle CSS, some cleanup)

Thank you again for your patience and guidance, and for sharing your screenshot! It's exciting to see RTL support coming together across the ecosystem.

Screenshot-menu-control

FIXES:
- Fix getVisibleRange RTL scrollLeft sign convention (enables horizontal scrolling)
- Add self-contained RTL container with dir='rtl' and .slick-rtl class
- Add applyRTL() helper for consistent state management
- Move resize-handle CSS from example to library (remove !important)
- Expand demo to 12 columns (forces horizontal scrolling)
- Change charset from iso-8859-1 to UTF-8
- Add 15 RTL Cypress tests

KNOWN LIMITATIONS:
- Frozen columns overlap in RTL (Out of Scope)
- scrollCellIntoView() API needs further work
- Runtime toggling needs additional testing
- Grid Menu/Column Picker full RTL styling
- CSS logical properties to reduce branching
- Auto-detection of direction from container
- Complete runtime toggling support
@jahanbakhsh18

Copy link
Copy Markdown
Author

Hi @6pac and @ghiscoding,
I've pushed a new commit addressing several issues from previous reviews.

  • Fix getVisibleRange RTL scrollLeft sign convention (enables horizontal scrolling)
  • 15 Cypress tests added for RTL functionality
  • Self-contained RTL container with dir="rtl" and .slick-rtl class
  • Resize-handle CSS moved from example to library
  • Demo expanded to 12 columns (forces horizontal scrolling)

Thanks again for the thorough review and guidance.
Here's a short demo showing the RTL grid in action with horizontal scrolling:

1271.mp4
Initial view - columns ordered from right to left Scrolled to the left - RTL scrolling works correctly
1 2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

slickGrid column resizing not worked Properly in RTL

3 participants