From fceffea5105abd1fb5cde5bcc3c8ea596675aab9 Mon Sep 17 00:00:00 2001 From: Peter Yates Date: Fri, 6 Jan 2023 15:07:34 +0000 Subject: [PATCH 1/2] Fix call to Logger#warning with Logger#warn --- .../govuk_component/table_component/cell_component.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/components/govuk_component/table_component/cell_component.rb b/app/components/govuk_component/table_component/cell_component.rb index 4d5cdf14..d141c233 100644 --- a/app/components/govuk_component/table_component/cell_component.rb +++ b/app/components/govuk_component/table_component/cell_component.rb @@ -59,7 +59,7 @@ def determine_scope in { auto_table_scopes: true, parent: 'tbody' } 'row' else - Rails.logger.warning("No scope pattern matched") + Rails.logger.warn("No scope pattern matched") nil end From 8d1dbfbde37d1af53e5222457b517f2fe54873b3 Mon Sep 17 00:00:00 2001 From: Peter Yates Date: Fri, 6 Jan 2023 15:27:20 +0000 Subject: [PATCH 2/2] Set table header default default by location This setting allows us to remove the header param from several places as we can tidily infer whether a cell should be a `th` or a `td` by whether its parent is a `thead` or a `tbody` while still respecting the first_cell_is_header setting and any manual overrides. The default is applied at the cell level whenever `header: nil`, when it's truthy or falsy the override will take precedence. --- .../table_component/cell_component.rb | 8 ++++++-- .../table_component/head_component.rb | 5 ++--- .../table_component/row_component.rb | 13 ++++++++----- guide/lib/examples/table_helpers.rb | 14 +++++++------- .../govuk_component/table_component_spec.rb | 18 ++++++++++++++++-- 5 files changed, 39 insertions(+), 19 deletions(-) diff --git a/app/components/govuk_component/table_component/cell_component.rb b/app/components/govuk_component/table_component/cell_component.rb index d141c233..fa063d05 100644 --- a/app/components/govuk_component/table_component/cell_component.rb +++ b/app/components/govuk_component/table_component/cell_component.rb @@ -13,13 +13,13 @@ class GovukComponent::TableComponent::CellComponent < GovukComponent::Base "one-quarter" => "govuk-!-width-one-quarter", }.freeze - def initialize(scope: nil, header: false, numeric: false, text: nil, width: nil, parent: nil, classes: [], html_attributes: {}) - @header = header + def initialize(scope: nil, header: nil, numeric: false, text: nil, width: nil, parent: nil, classes: [], html_attributes: {}) @text = text @numeric = numeric @width = width @scope = scope @parent = parent + @header = (header.nil?) ? in_thead? : header super(classes: classes, html_attributes: html_attributes) end @@ -76,4 +76,8 @@ def default_classes def width_class WIDTHS.fetch(width, nil) end + + def in_thead? + parent == "thead" + end end diff --git a/app/components/govuk_component/table_component/head_component.rb b/app/components/govuk_component/table_component/head_component.rb index 1dd843b3..cd06f257 100644 --- a/app/components/govuk_component/table_component/head_component.rb +++ b/app/components/govuk_component/table_component/head_component.rb @@ -1,8 +1,7 @@ class GovukComponent::TableComponent::HeadComponent < GovukComponent::Base - renders_many :rows, ->(cell_data: nil, header: true, classes: [], html_attributes: {}, &block) do + renders_many :rows, ->(cell_data: nil, classes: [], html_attributes: {}, &block) do GovukComponent::TableComponent::RowComponent.from_head( cell_data: cell_data, - header: header, classes: classes, html_attributes: html_attributes, &block @@ -26,7 +25,7 @@ def call def build_rows_from_row_data(data) return if data.blank? - data.each { |d| row(cell_data: d, header: true) } + data.each { |d| row(cell_data: d) } end def default_attributes diff --git a/app/components/govuk_component/table_component/row_component.rb b/app/components/govuk_component/table_component/row_component.rb index 000b9df4..984df894 100644 --- a/app/components/govuk_component/table_component/row_component.rb +++ b/app/components/govuk_component/table_component/row_component.rb @@ -1,5 +1,5 @@ class GovukComponent::TableComponent::RowComponent < GovukComponent::Base - renders_many :cells, ->(scope: nil, header: false, text: nil, numeric: false, width: nil, classes: [], html_attributes: {}, &block) do + renders_many :cells, ->(scope: nil, header: nil, text: nil, numeric: false, width: nil, classes: [], html_attributes: {}, &block) do GovukComponent::TableComponent::CellComponent.new( scope: scope, header: header, @@ -13,10 +13,9 @@ class GovukComponent::TableComponent::RowComponent < GovukComponent::Base ) end - attr_reader :header, :first_cell_is_header, :parent + attr_reader :first_cell_is_header, :parent - def initialize(cell_data: nil, first_cell_is_header: false, header: false, parent: nil, classes: [], html_attributes: {}) - @header = header + def initialize(cell_data: nil, first_cell_is_header: false, parent: nil, classes: [], html_attributes: {}) @first_cell_is_header = first_cell_is_header @parent = parent @@ -52,10 +51,14 @@ def cell_attributes(count) end def cell_is_header?(count) - header || (first_cell_is_header && count.zero?) + in_thead? || (first_cell_is_header && count.zero?) end def default_attributes { class: %w(govuk-table__row) } end + + def in_thead? + parent == "thead" + end end diff --git a/guide/lib/examples/table_helpers.rb b/guide/lib/examples/table_helpers.rb index a5177cf2..377a043f 100644 --- a/guide/lib/examples/table_helpers.rb +++ b/guide/lib/examples/table_helpers.rb @@ -7,9 +7,9 @@ def table_normal - table.head do |head| - head.row do |row| - - row.cell(header: true, text: 'Name') - - row.cell(header: true, text: 'Types') - - row.cell(header: true, text: 'Pokédex number', numeric: true) + - row.cell(text: 'Name') + - row.cell(text: 'Types') + - row.cell(text: 'Pokédex number', numeric: true) - table.body do |body| - body.row do |row| @@ -36,8 +36,8 @@ def table_with_header_column - table.head do |head| - head.row do |row| - - row.cell(header: true, text: 'Generation') - - row.cell(header: true, text: 'Years') + - row.cell(text: 'Generation') + - row.cell(text: 'Years') - table.body do |body| - body.row do |row| @@ -85,8 +85,8 @@ def table_with_resized_columns - table.head do |head| - head.row do |row| - - row.cell(header: true, text: 'Name', width: 'one-third') - - row.cell(header: true, text: 'Description') + - row.cell(text: 'Name', width: 'one-third') + - row.cell(text: 'Description') - table.body do |body| - body.row do |row| diff --git a/spec/components/govuk_component/table_component_spec.rb b/spec/components/govuk_component/table_component_spec.rb index b5530a42..4b9ff10c 100644 --- a/spec/components/govuk_component/table_component_spec.rb +++ b/spec/components/govuk_component/table_component_spec.rb @@ -12,11 +12,17 @@ table.caption(text: "What a nice table") table.head do |head| - head.row {} + head.row do |row| + row.cell(text: "A") + row.cell(text: "B") + end end table.body do |body| - body.row {} + body.row do |row| + row.cell(text: "C") + row.cell(text: "D") + end end end end @@ -36,10 +42,18 @@ expect(rendered_content).to have_tag("table > thead") end + specify "the cells in thead should default to th" do + expect(rendered_content).to have_tag("table > thead > tr > th", count: 2) + end + specify "renders a tbody element" do expect(rendered_content).to have_tag("table > tbody") end + specify "the cells in tbody should default to td" do + expect(rendered_content).to have_tag("table > tbody > tr > td", count: 2) + end + context "when there is more than one tbody" do let(:expected_count) { 2 }