[DREAM-858] Index Border Box Table rows for ARIA by myabc · Pull Request #25584 · opf/openproject

myabc · 2026-09-24T15:34:39Z

Ticket

https://community.openproject.org/wp/DREAM-858

What are you trying to accomplish?

OpPrimer::BorderBoxTableComponent renders an ARIA table with column indices but no row indices, so screen readers cannot announce a row's position within a paginated collection and specs cannot address rows through the accessible :row selector's rowindex: filter.

The table now sets aria-rowcount and every row carries aria-rowindex: both header rows share index 1 (only one is displayed per breakpoint), body rows start at 2 offset by the current page, and the footer row is last.

What approach did you choose and why?

The attributes are additive, so consumers need no change. aria-rowcount uses total_entries for paginated collections and the rendered size otherwise; the page offset comes from current_page and per_page, which both WillPaginate::Collection and paginated relations expose.

sortable-lists moves rows on the client before the server answers (the Roles table keeps a 204 response), so reorderRows and restoreRowPositions now run through reindexAriaRowsAfter in the new sortable-lists/aria-row-indices.ts, which renumbers the indexed rows of a container in DOM order after a move or rollback. Scoped to reordering within one table; lists without aria-rowindex are untouched.

Pages::Admin::Statuses#expect_listed and Pages::Admin::DocumentTypes#expect_order assert through rowindex:, which replaces the :row position: patch added in #25468.

AI involvement

Collaborative – AI generated a substantial part of the code; I reviewed and understand every line.

Merge checklist

Added/updated tests
Added/updated documentation in Lookbook (patterns, previews, etc)
Tested major browsers (Chrome, Firefox, Edge, ...)

github-actions · 2026-09-24T15:37:08Z

	1 Warning
⚠️	@opf/dream-team Files in `app/components/op_primer` were modified: app/components/op_primer/border_box_table_component.html.erb app/components/op_primer/border_box_table_component.rb Please review these changes to ensure they align with the design system guidelines.

Generated by 🚫 Danger

Copilot

Copilot review overview

🟡 Changes recommended

Out-of-range paginated pages can report incorrect row counts and footer indices.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

Use total_entries for paginated table row count · New

What changed in this PR

Adds ARIA row counts and indices to Border Box tables, including sortable-list reindexing and test/documentation coverage.

Changes:

Adds pagination-aware aria-rowcount and aria-rowindex.
Reindexes rows after sorting and rollback.
Updates component, frontend, feature, and LookBook coverage.

File	Description
`spec/features/roles/index_spec.rb`	Verifies accessible row indices.
`spec/components/op_primer/border_box_table_component_spec.rb`	Tests ARIA table semantics and pagination.
`lookbook/docs/10-components/tables/border-box-table.md.erb`	Documents ARIA indexing support.
`frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts`	Integrates row reindexing with sorting.
`frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts`	Tests reorder and rollback behavior.
`frontend/src/stimulus/controllers/dynamic/sortable-lists/aria-row-indices.ts`	Provides sortable-row reindexing.
`app/components/op_primer/border_box_table_component.rb`	Calculates row counts and pagination offsets.
`app/components/op_primer/border_box_table_component.html.erb`	Renders ARIA row attributes.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Copilot review overview

🔵 Needs a closer look

Two moderate issues remain regarding empty-state row indexing and avoidable relation queries.

Review effort: Lite
Findings: None

Resolved since last review (1)

Use total_entries for paginated table row count

bsatarnejad · 2026-09-30T12:34:32Z

+    end
+
+    def body_row_count
+      [paginated? ? rows.total_entries : rows.size, 1].max


Could we use rows.length for the non-paginated case? We render all these records anyway, so this would avoid a separate count query before loading them.

size should take care of that automatically - calling length if the collection is loaded and count if not.

Rows of the shared table carried no aria-rowindex, so assistive technology could not announce a row's position within a paginated collection and specs could not address rows by position through the accessible :row selector's rowindex filter. Sets aria-rowcount on the table and aria-rowindex on every row, offset by the current page. Both header rows share index 1 because only one is displayed per breakpoint. https://community.openproject.org/wp/DREAM-858

sortable-lists moves rows on the client before the server answers, and the Roles table keeps a 204 response, so the server-rendered aria-rowindex stayed with the moved record and the table announced positions out of order. Captures the first index of every touched rows container before a move or rollback and renumbers the indexed rows in DOM order afterwards, which keeps the page offset. The Roles feature spec asserts row indices whenever it checks the listing. https://community.openproject.org/wp/DREAM-858

Replaces the link-text scrape in Pages::Admin::Statuses#expect_listed with the accessible :row selector's rowindex filter, so the listing checks the announced positions too. Page-two examples pass the first row's index, which proves the page offset in the live table.

Clears the token association cache before the API and iCalendar meeting creation requests in DREAM-858 specs. ARIA row counting calls size, which marks empty associations as loaded, while login_as reuses the same user across requests. Reloading matches production request boundaries and preserves the post-creation table assertions.

Asserts the document types order through the `:row` selector's `rowindex:` filter now that Border Box Table rows carry `aria-rowindex`, and drops the `position:` expression filter patched onto `:row` for lack of it. https://community.openproject.org/wp/DREAM-858

github-actions · 2026-09-30T14:44:16Z

Warning

Flaky specs

rspec ./modules/documents/spec/features/documents/project/show_edit_document_spec.rb[1:1]
rspec ./modules/overviews/spec/features/managing_dashboard_page_spec.rb[1:1:1]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #25584, linked for reference only):

- `rspec ./modules/documents/spec/features/documents/project/show_edit_document_spec.rb[1:1]`
- `rspec ./modules/overviews/spec/features/managing_dashboard_page_spec.rb[1:1:1]`

Treat this as a standalone task, unrelated to PR #25584. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #25584 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @myabc to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @myabc, and request a review from @myabc.
On every commit, set @myabc as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

myabc · 2026-09-30T16:49:17Z

@bsatarnejad I've also pushed one new commit that needs review: 1773d47) (it's just spec cleanup left over from #25468, so I went ahead and did it in this PR)

myabc requested a lite review from Copilot September 24, 2026 15:35

Copilot started reviewing on behalf of myabc September 24, 2026 15:36 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

Comment thread app/components/op_primer/border_box_table_component.rb Outdated

github-actions Bot added the ai: Collaborative 💻 AI generated a substantial part of the code; A human reviewed and understands every line. label Sep 24, 2026

myabc added Accessibility needs review labels Sep 24, 2026

myabc added this to the 18.0.x milestone Sep 24, 2026

myabc force-pushed the code-maintenance/dream-858-border-box-table-row-indices branch from 084a13d to e25645b Compare September 24, 2026 15:46

myabc marked this pull request as ready for review September 24, 2026 15:46

myabc force-pushed the code-maintenance/dream-858-border-box-table-row-indices branch from e25645b to d0ab302 Compare September 24, 2026 15:48

myabc added ruby Pull requests that update Ruby code javascript Pull requests that update Javascript code labels Sep 24, 2026

myabc mentioned this pull request Sep 24, 2026

[DREAM-840] Show document types in a table #25468

Merged

1 of 3 tasks

myabc force-pushed the code-maintenance/dream-858-border-box-table-row-indices branch from d0ab302 to 7382936 Compare September 24, 2026 17:30

myabc requested a review from bsatarnejad September 25, 2026 07:55

myabc force-pushed the code-maintenance/dream-858-border-box-table-row-indices branch 2 times, most recently from 49f765b to a27a663 Compare September 25, 2026 18:41

myabc requested a lite review from Copilot September 25, 2026 18:44

Copilot started reviewing on behalf of myabc September 25, 2026 18:45 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

bsatarnejad requested changes Sep 30, 2026

View reviewed changes

myabc added 4 commits September 30, 2026 15:06

myabc requested a review from bsatarnejad September 30, 2026 14:18

myabc force-pushed the code-maintenance/dream-858-border-box-table-row-indices branch from a27a663 to dd4be4d Compare September 30, 2026 14:18

Navigation Menu

[DREAM-858] Index Border Box Table rows for ARIA - #25584

[DREAM-858] Index Border Box Table rows for ARIA#25584
myabc wants to merge 5 commits into
devfrom
code-maintenance/dream-858-border-box-table-row-indices

myabc commented Sep 24, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

bsatarnejad Sep 30, 2026

Uh oh!

myabc Sep 30, 2026

Uh oh!

github-actions Bot commented Sep 30, 2026

Uh oh!

myabc commented Sep 30, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

Conversation

myabc commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ticket

What are you trying to accomplish?

What approach did you choose and why?

AI involvement

Merge checklist

Uh oh!

github-actions Bot commented Sep 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Uh oh!

bsatarnejad Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

myabc Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 30, 2026

Uh oh!

myabc commented Sep 30, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

myabc commented Sep 24, 2026 •

edited

Loading