[#70204] Introduce `DataTableComponent` (port from Primer React) and unstyled `TableComponent`. by myabc · Pull Request #21532 · opf/openproject

myabc · 2026-01-03T00:51:49Z

Ticket

https://community.openproject.org/wp/70204

What are you trying to accomplish?

Screenshots

What approach did you choose and why?

Prior art

https://github.com/x-govuk/govuk-components/tree/main/app/components/govuk_component/table_component

Merge checklist

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

github-actions · 2026-01-03T20:36:56Z

Caution

The provided work package version does not match the core version

Details:

Work package URL: https://community.openproject.org/wp/70204
Work package version: not set
Core version: 17.1.0

Please make sure that:

The work package version OR your pull request target branch is correct

myabc · 2026-01-03T20:37:08Z

+  <%= title %>
+  <%= subtitle %>
+  <%= render(Primer::BaseComponent.new(**@wrapper_arguments)) do %>
+    <data-table-element2>


Fix Catalyst issue.

Copilot

Pull request overview

This PR introduces a new table component architecture for OpenProject using the Primer design system. It adds OpPrimer::TableComponent (a low-level semantic HTML table wrapper) and OpPrimer::DataTableComponent (a higher-level data-driven table with sorting), along with client-side sorting functionality via a custom web component. Existing table components are refactored to use these new primitives.

Key changes:

New OpPrimer::TableComponent with nested sub-components (Head, Body, Row, Cell, etc.) for building accessible tables
New OpPrimer::DataTableComponent for rendering data collections with client-side sorting
Custom data-table-element web component for interactive table sorting
Refactored TableComponent and Projects::TableComponent to use the new Primer components
Comprehensive RSpec test coverage and Lookbook previews

Reviewed changes

Copilot reviewed 51 out of 52 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
`frontend/src/elements/data-table-element.ts`	Implements client-side table sorting via Catalyst controller
`frontend/src/main.ts`	Registers the new data-table-element custom component
`app/components/op_primer/table_component.rb`	Core table component with slots for caption, colgroup, head, body, foot
`app/components/op_primer/table_component/*.rb`	Sub-components for table structure (HeaderComponent, CellComponent, RowComponent, etc.)
`app/components/op_primer/data_table_component.rb`	Data-driven table with column definitions and sorting
`app/components/op_primer/data_table_component/*.rb`	Column and sort header sub-components
`app/components/op_primer/data_table_component.scss`	Primer-based table styling with grid layout
`app/components/table_component.html.erb`	Refactored to use `OpPrimer::TableComponent`
`app/components/row_component.html.erb`	Refactored to use `OpPrimer::TableComponent::RowComponent`
`app/components/projects/table_component.html.erb`	Refactored to use `OpPrimer::TableComponent`
`app/components/projects/row_component.html.erb`	Refactored to use `OpPrimer::TableComponent::RowComponent`
`app/components/op_primer/border_box_table_component.rb`	Updated to use `::TableComponent` to avoid naming collision
`app/components/op_primer/border_box_row_component.rb`	Updated to use `::RowComponent` to avoid naming collision
`app/helpers/sort_helper.rb`	Added `with_th` option to allow rendering sort headers without wrapping `<th>`
`lookbook/previews/op_primer/table_component_preview.rb`	Lookbook preview for basic table component
`lookbook/previews/op_primer/data_table_component_preview.rb`	Lookbook preview for data table with examples
`spec/components/op_primer/table_component_spec.rb`	Comprehensive tests for table component
`spec/components/op_primer/data_table_component_spec.rb`	Tests for data table including title/subtitle accessibility
`spec/components/op_primer/table_component/*_spec.rb`	Unit tests for all table sub-components
`spec/helpers/sort_helper_spec.rb`	Test for new `with_th: false` option
`spec/views/users/index.html.erb_spec.rb`	Updated test expectations for whitespace changes
`app/components/_index.sass`	Imports data_table_component styles

Copilot · 2026-01-03T20:40:12Z

+  <%= title %>
+  <%= subtitle %>
+  <%= render(Primer::BaseComponent.new(**@wrapper_arguments)) do %>
+    <data-table-element2>


The custom element name data-table-element2 appears to be a temporary or debugging name. The "2" suffix suggests this might be a second iteration or test version. Consider using a more descriptive and production-ready name like data-table or op-data-table to better follow naming conventions and avoid confusion.

Copilot · 2026-01-03T20:40:13Z

+    render(
+      Primer::Beta::BaseButton.new(
+        classes: "ButtonReset TableSortButton",
+        data: { action: "click:data-table-element2#toggleSort" }


The data attribute name data-action="click:data-table-element2#toggleSort" references data-table-element2, which should be updated to match the actual custom element name. Additionally, ensure this matches the controller decorator used in the TypeScript file.

Suggested change

data: { action: "click:data-table-element2#toggleSort" }

data: { action: "click:data-table-element#toggleSort" }

Copilot · 2026-01-03T20:40:13Z

+/*
+ * -- copyright
+ * OpenProject is an open source project management software.
+ * Copyright (C) 2023 the OpenProject GmbH


The copyright year in this newly added file is "2023", but according to the PR context, it should be "2025" or the current year when the file was created. This inconsistency should be corrected to match other recently added files in the PR.

Copilot · 2026-01-03T20:40:13Z

+      if @initial_sort_column
+        column = columns.find { |column| column.id == @initial_sort_column || column.field == @initial_sort_column }
+        raise ArgumentError, "Invalid Sort column" unless column
+        raise ArgumentError, "Invallid Sortby" unless column.sort_by


There's a typo in the error message: "Invallid Sortby" should be "Invalid sort_by". This error message would be confusing to developers debugging sort configuration issues.

Suggested change

raise ArgumentError, "Invallid Sortby" unless column.sort_by

raise ArgumentError, "Invalid sort_by" unless column.sort_by

Copilot · 2026-01-03T20:40:13Z

+  <% colgroups.each do |colgroup| %>
+    <%= colgroup %>
+  <% end %>
+  <% cols.each do |cols| %>


There's a typo in the variable name: cols should be col in this iteration block. This would cause a NameError at runtime since the loop variable is named col but the code references cols.

Suggested change

<% cols.each do |cols| %>

<% cols.each do |col| %>

Copilot · 2026-01-03T20:40:14Z

+//if (!customElements.get('data-table')) {
+//  customElements.define('data-table', DataTableElement);
+//}


Commented-out code should be removed before merging. These lines appear to be leftover debug or development code that should either be uncommented if needed or deleted entirely to keep the codebase clean.

Copilot · 2026-01-03T20:40:14Z

+  constructor() {
+    super();
+  }


The empty constructor is redundant and can be removed. In TypeScript, if a constructor doesn't perform any initialization beyond calling super(), it can be omitted entirely as the default constructor will be used automatically.

Copilot · 2026-01-03T20:40:14Z

+  const sortedRows = rows.sort((a, b) => {
+    const aText = a.children[columnIndex].textContent?.trim() ?? '';
+    const bText = b.children[columnIndex].textContent?.trim() ?? '';
+
+    const aNum = parseFloat(aText);
+    const bNum = parseFloat(bText);
+
+    const valueA = isNaN(aNum) ? aText : aNum;
+    const valueB = isNaN(bNum) ? bText : bNum;
+
+    if (valueA < valueB) return direction === 'ascending' ? -1 : 1;
+    if (valueA > valueB) return direction === 'ascending' ? 1 : -1;
+    return 0;
+  });


The sorting logic uses simple string and numeric comparison which may not handle edge cases well. Consider: 1) Dates may not sort correctly if not in ISO format, 2) Mixed alphanumeric values (e.g., "Project 2" vs "Project 10") won't sort naturally, 3) Case-sensitive string comparison may produce unexpected ordering. Consider using localeCompare for string comparisons and more robust date/number parsing.

Copilot · 2026-01-03T20:40:14Z

+      sortDescendingIcon?.classList.add('d-none'); 
+    } else {
+      header.setAttribute('aria-sort', 'descending');
+      sortDescendingIcon?.classList.remove('d-none');
+      sortAscendingIcon?.classList.add('d-none'); 
+    }
+
+    const siblings = [...header.parentElement!.children].filter(el => el !== header);
+    siblings.forEach((sibling:HTMLElement) => {
+      resetSort(sibling);
+    });
+
+    sortTableByAriaSort(this.table);
+  }
+
+  get table():HTMLTableElement {
+    return this.querySelector('table')!;
+  }
+}
+
+
+function resetSort(th:HTMLElement) {
+  th.removeAttribute('aria-sort');
+  const sortAscendingIcon = th.querySelector('.TableSortIcon--ascending');
+  const sortDescendingIcon = th.querySelector('.TableSortIcon--descending');
+  sortAscendingIcon?.classList.remove('d-none');
+  sortDescendingIcon?.classList.add('d-none'); 


Trailing whitespace is present after several semicolons in this file. While this doesn't affect functionality, it violates code style consistency. Consider running a linter to remove trailing whitespace throughout the file.

Initial port of Primer React's DataTable component to View Component. Also introduces `OpPrimer::TableCompoonent`, an unstyled table component used by `DataTableComponent` under-the-hood. See: https://primer.style/product/components/data-table/ https://community.openproject.org/wp/70204

myabc · 2026-03-08T02:55:04Z

Replaced by upstream PR: opf/primer_view_components#409

myabc commented Jan 3, 2026

View reviewed changes

Comment thread spec/helpers/sort_helper_spec.rb Outdated

myabc force-pushed the feature/70204-data-table-component branch from 9201a7d to 42b583c Compare January 3, 2026 20:34

myabc marked this pull request as ready for review January 3, 2026 20:36

Copilot AI review requested due to automatic review settings January 3, 2026 20:36

Copilot started reviewing on behalf of myabc January 3, 2026 20:37 View session

myabc commented Jan 3, 2026

View reviewed changes

Copilot AI reviewed Jan 3, 2026

View reviewed changes

myabc changed the title ~~Feature/70204 data table component~~ [#70204] Introduce DataTableComponent (port from Primer React) and unstyled TableComponent. Jan 3, 2026

myabc added DO NOT MERGE Accessibility feature javascript Pull requests that update Javascript code styling ruby Pull requests that update Ruby code labels Jan 3, 2026

myabc added 3 commits January 4, 2026 04:43

Make RowGroupComponent accept content and slots

b9d6800

Rework legacy tables with OpPrimer::TableComponent

f04d829

myabc force-pushed the feature/70204-data-table-component branch from 42b583c to f04d829 Compare January 4, 2026 07:43

myabc closed this Mar 8, 2026

myabc deleted the feature/70204-data-table-component branch March 8, 2026 02:55

github-actions Bot locked and limited conversation to collaborators Mar 8, 2026

	data: { action: "click:data-table-element2#toggleSort" }
	data: { action: "click:data-table-element#toggleSort" }

	raise ArgumentError, "Invallid Sortby" unless column.sort_by
	raise ArgumentError, "Invalid sort_by" unless column.sort_by

Conversation

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

Uh oh!

Ticket

What are you trying to accomplish?

Screenshots

What approach did you choose and why?

Prior art

Merge checklist

Uh oh!

Uh oh!

github-actions Bot commented Jan 3, 2026

Uh oh!

myabc Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

myabc commented Mar 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

myabc commented Jan 3, 2026 •

edited

Loading