Skip to content

Commit dc5f3c4

Browse files
committed
Adds DataTable row_id for addressable rows
Adds a row_id proc that emits a data-row-id attribute on each body row, plus an opt-in row_dom_id flag that also assigns a DOM id namespaced by the table id, keeping ids unique across tables. Rows whose proc returns a blank value get no attributes. Diverges from Primer React's getRowId, which only feeds virtual DOM keys: here the identifier addresses rows in the rendered DOM, e.g. for Turbo Stream targets or test selectors.
1 parent 2a6e7cb commit dc5f3c4

6 files changed

Lines changed: 97 additions & 2 deletions

File tree

app/components/primer/open_project/data_table.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
<% end %>
3838
<% table.with_body(classes: "TableBody") do |tbody| %>
3939
<% rows.each do |row| %>
40-
<% tbody.with_row(classes: "TableRow") do |tr| %>
40+
<% tbody.with_row(**row_arguments(row)) do |tr| %>
4141
<% cells_for(row).each do |cell| %>
4242
<% cell_arguments = { classes: "TableCell", align: cell.column.align } %>
4343
<% cell_arguments[:data] = cell.sort_data if cell.sortable? %>

app/components/primer/open_project/data_table.rb

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,15 @@ def sort_data
112112
# @param divider [Boolean]
113113
# Whether to render a presentational divider line below the title row,
114114
# ported from Primer React's `Table.Divider`
115+
# @param row_id [Proc, nil]
116+
# Optional `->(row)` returning an identifier for each row, emitted as a
117+
# `data-row-id` attribute on the row's `<tr>`. Unlike Primer React's
118+
# `getRowId` (which feeds virtual-DOM keys), this addresses rows in the
119+
# DOM, e.g. for Turbo Stream targets or test selectors. Rows for which
120+
# the proc returns a blank value get no attribute.
121+
# @param row_dom_id [Boolean]
122+
# Whether each `<tr>` additionally gets a DOM `id`, namespaced by the
123+
# table id to stay unique across tables. Requires `row_id`.
115124
# @param html_data [Hash]
116125
# HTML data attributes to be passed to the table
117126
# @param system_arguments [Hash]
@@ -122,6 +131,8 @@ def initialize(
122131
initial_sort_column: nil,
123132
initial_sort_direction: nil,
124133
divider: false,
134+
row_id: nil,
135+
row_dom_id: false,
125136
html_data: {},
126137
**system_arguments
127138
)
@@ -130,6 +141,9 @@ def initialize(
130141
@initial_sort_column = initial_sort_column
131142
@initial_sort_direction = initial_sort_direction
132143
@divider = fetch_or_fallback_boolean(divider, false)
144+
@row_id_proc = row_id
145+
@row_dom_id = fetch_or_fallback_boolean(row_dom_id, false)
146+
raise ArgumentError, "`row_dom_id` requires a `row_id` proc" if @row_dom_id && @row_id_proc.nil?
133147
@id = system_arguments[:id] ||= self.class.generate_id(base_name: "data-table")
134148

135149
@container_arguments = {}
@@ -286,6 +300,18 @@ def grid_template_from_columns(columns)
286300
end
287301
end
288302

303+
def row_arguments(row)
304+
arguments = { classes: "TableRow" }
305+
return arguments unless @row_id_proc
306+
307+
value = @row_id_proc.call(row).to_s
308+
return arguments if value.blank?
309+
310+
arguments[:data] = { row_id: value }
311+
arguments[:id] = "#{@id}-row-#{value}" if @row_dom_id
312+
arguments
313+
end
314+
289315
def cell_content(cell, row)
290316
value = cell.column.render_cell(row).to_s
291317
return value if value.present? || cell.column.placeholder.blank?

previews/primer/open_project/data_table_preview/playground.html.erb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
rows,
55
cell_padding: cell_padding,
66
initial_sort_column: initial_sort_column,
7-
initial_sort_direction: initial_sort_direction
7+
initial_sort_direction: initial_sort_direction,
8+
row_id: ->(row) { row.id }
89
)
910
) do |data_table|
1011
%>

static/arguments.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6094,6 +6094,18 @@
60946094
"default": "`false`",
60956095
"description": "Whether to render a presentational divider line below the title row, ported from Primer React's `Table.Divider`"
60966096
},
6097+
{
6098+
"name": "row_id",
6099+
"type": "Proc, nil",
6100+
"default": "`nil`",
6101+
"description": "Optional `->(row)` returning an identifier for each row, emitted as a `data-row-id` attribute on the row's `<tr>`. Unlike Primer React's `getRowId` (which feeds virtual-DOM keys), this addresses rows in the DOM, e.g. for Turbo Stream targets or test selectors. Rows for which the proc returns a blank value get no attribute."
6102+
},
6103+
{
6104+
"name": "row_dom_id",
6105+
"type": "Boolean",
6106+
"default": "`false`",
6107+
"description": "Whether each `<tr>` additionally gets a DOM `id`, namespaced by the table id to stay unique across tables. Requires `row_id`."
6108+
},
60976109
{
60986110
"name": "html_data",
60996111
"type": "Hash",

static/info_arch.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20011,6 +20011,18 @@
2001120011
"default": "`false`",
2001220012
"description": "Whether to render a presentational divider line below the title row, ported from Primer React's `Table.Divider`"
2001320013
},
20014+
{
20015+
"name": "row_id",
20016+
"type": "Proc, nil",
20017+
"default": "`nil`",
20018+
"description": "Optional `->(row)` returning an identifier for each row, emitted as a `data-row-id` attribute on the row's `<tr>`. Unlike Primer React's `getRowId` (which feeds virtual-DOM keys), this addresses rows in the DOM, e.g. for Turbo Stream targets or test selectors. Rows for which the proc returns a blank value get no attribute."
20019+
},
20020+
{
20021+
"name": "row_dom_id",
20022+
"type": "Boolean",
20023+
"default": "`false`",
20024+
"description": "Whether each `<tr>` additionally gets a DOM `id`, namespaced by the table id to stay unique across tables. Requires `row_id`."
20025+
},
2001420026
{
2001520027
"name": "html_data",
2001620028
"type": "Hash",

test/components/primer/open_project/data_table/data_table_test.rb

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,50 @@ def test_renders_no_divider_by_default
460460
assert_no_selector(".TableDivider")
461461
end
462462

463+
def test_emits_data_row_id_with_row_id_proc
464+
render_component(@data, row_id: ->(row) { row.id }) do |data_table|
465+
data_table.with_column(field: :subject, header: "Subject")
466+
end
467+
468+
assert_selector("tbody tr[data-row-id='1']")
469+
assert_selector("tbody tr[data-row-id='3']")
470+
assert_no_selector("tbody tr[id]")
471+
end
472+
473+
def test_emits_namespaced_dom_id_with_row_dom_id
474+
render_component(@data, id: "my-table", row_id: ->(row) { row.id }, row_dom_id: true) do |data_table|
475+
data_table.with_column(field: :subject, header: "Subject")
476+
end
477+
478+
assert_selector("tbody tr#my-table-row-1[data-row-id='1']")
479+
end
480+
481+
def test_emits_no_row_attributes_without_row_id_proc
482+
render_component(@data) do |data_table|
483+
data_table.with_column(field: :subject, header: "Subject")
484+
end
485+
486+
assert_no_selector("tbody tr[data-row-id]")
487+
end
488+
489+
def test_skips_row_attributes_for_blank_row_id
490+
row_klass = Data.define(:id, :subject)
491+
data = [row_klass.new(id: nil, subject: "First"), row_klass.new(id: 2, subject: "Second")]
492+
493+
render_component(data, row_id: ->(row) { row.id }) do |data_table|
494+
data_table.with_column(field: :subject, header: "Subject")
495+
end
496+
497+
assert_selector("tbody tr[data-row-id='2']")
498+
assert_selector("tbody tr[data-row-id]", count: 1)
499+
end
500+
501+
def test_raises_for_row_dom_id_without_row_id
502+
assert_raises(ArgumentError) do
503+
Primer::OpenProject::DataTable.new(@data, row_dom_id: true)
504+
end
505+
end
506+
463507
def test_renders_default_empty_state_without_rows
464508
render_component([]) do |data_table|
465509
data_table.with_column(field: :subject, header: "Subject")

0 commit comments

Comments
 (0)