Skip to content

Floating table actions - #108

Merged
thomasgallagher merged 4 commits into
mainfrom
floating-table-actions
Jul 9, 2026
Merged

Floating table actions#108
thomasgallagher merged 4 commits into
mainfrom
floating-table-actions

Conversation

@thomasgallagher

Copy link
Copy Markdown
Collaborator

No description provided.

@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review

Adds a horizontal scroll hint controller () that fades the clipped edges of a scrollable region while more content is available, and a option on that pins the actions column to the right edge via positioning. Both features are auto-wired in the helper and documented in the dummy app.

No issues found.

Ship - Clean, well-tested implementation with correct keyboard accessibility (tabindex management, WCAG 2.1.1) and accurate focus-ring styling on the scroll container.

@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown

Floating table actions - scroll hint + floating column

Adds a horizontal scroll-hint controller that fades clipped edges of a scroller, and a floating_actions: true option for l_ui_table that pins the actions column sticky-right while the table scrolls horizontally.

Issues

Severity File Line(s) Description
MEDIUM scroll_hint_controller.js 9 this.update = this.update.bind(this) is called inside connect(), not initialize(). Stimulus calls connect() again on every Turbo reconnection while reusing the same controller instance. Each reconnect replaces this.update with a new wrapper around the previously-bound function, building a call chain one level deeper per navigation. initialize() runs once per instance and is the correct place for one-time binding. Fix: move the bind call (and only the bind call) to initialize() { this.update = this.update.bind(this) }.

Verdict

🟧 Ship (with known minor issue) - The functional logic, CSS, accessibility handling (WCAG 2.1.1 tabindex, 2.4.11 scroll-pr), and test coverage are all solid. The bind-in-connect pattern is a well-known Stimulus anti-pattern that grows a call chain on repeated Turbo navigations but causes no observable breakage in typical use; worth fixing before this controller sees heavy reconnect traffic.

@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review

Adds a sticky floating actions column that pins row action cells to the right edge of the scroll container while the table scrolls horizontally, plus a scroll hint controller that fades the clipped edges of any horizontal scroller to signal overflow.

No issues found. The implementation is clean: the controller lifecycle is correct (scroll/resize listeners registered once in initialize/connect, fully torn down in disconnect), the 1 px tolerance for fractional scroll positions is reasonable, floating_actions is correctly gated on actions being present before applying the modifier class, and the WCAG 2.1.1 tabindex="0" / 2.4.11 scroll-pr-20 considerations are well-handled.

Ship

@thomasgallagher
thomasgallagher merged commit a9c0938 into main Jul 9, 2026
4 checks passed
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.

1 participant