From 8e1753141fa299707f87ed4b240f90cbd033c299 Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Mon, 17 Feb 2025 17:37:20 +0000 Subject: [PATCH 01/17] Set showAllValues to 'true' whether selecting multiple or single In March 2023, in #7378, it was decided to make 'multiple select' autocomplete occurrences use the `showAllValues` option by default. This means that all options are displayed as soon as the input is interacted with, without any additional search action required. The PR did not describe why 'single' inputs should not receive the same treatment. It is hypothesised that without showing users the available options (i.e. enabling `showAllValues`) then they will find it more difficult to know which value to search for and select. If we don't change this behaviour, then it means we can't offer feature parity in swapping component usage from 'select with search' over to 'autocomplete' (which is what we're trying to do in https://trello.com/c/pmbL2yId/3411-remove-the-selectwithsearch-component-from-whitehall to improve consistency within Whitehall). Note that the behaviour can still be overridden on a case by case basis as per the example in the YML file: ```yml autocomplete_configuration_options: showAllValues: false ``` --- app/views/components/_autocomplete.html.erb | 4 +++- .../content_block_manager/content_block/shared/_form.html.erb | 3 --- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/views/components/_autocomplete.html.erb b/app/views/components/_autocomplete.html.erb index d8a2efd125b..bbbb02fb851 100644 --- a/app/views/components/_autocomplete.html.erb +++ b/app/views/components/_autocomplete.html.erb @@ -4,7 +4,9 @@ error_items ||= [] aria = error_id if error_items.any? select ||= {} - autocomplete_configuration_options ||= {} + autocomplete_configuration_options ||= { + showAllValues: true, + } data_attributes ||= {} data_attributes[:module] ||= "" diff --git a/lib/engines/content_block_manager/app/views/content_block_manager/content_block/shared/_form.html.erb b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/shared/_form.html.erb index 1a090040abd..8daa338cbf2 100644 --- a/lib/engines/content_block_manager/app/views/content_block_manager/content_block/shared/_form.html.erb +++ b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/shared/_form.html.erb @@ -38,9 +38,6 @@ options: [""] + taggable_organisations_container, selected: @form.content_block_edition.edition_organisation&.organisation_id, }, - autocomplete_configuration_options: { - showAllValues: true, - }, } %> <%= render "govuk_publishing_components/components/textarea", { From 587b28fa6376b24199f817b77497d65f92b203c9 Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Mon, 17 Feb 2025 17:38:37 +0000 Subject: [PATCH 02/17] Remove unnecessary 'onConfirm' callback It looks as though this used to be used for GA4 tracking, but that was removed in f541cb99442eb0dc0071f9da14d90439cf3276e6. What's left is just the native JS/select `selected` manipulation which you'd expect to be defined within accessible-autocomplete-multiselect itself. --- .../javascripts/components/autocomplete.js | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/app/assets/javascripts/components/autocomplete.js b/app/assets/javascripts/components/autocomplete.js index 82cc4be6206..d08d95d000d 100644 --- a/app/assets/javascripts/components/autocomplete.js +++ b/app/assets/javascripts/components/autocomplete.js @@ -14,22 +14,7 @@ window.GOVUK.Modules = window.GOVUK.Modules || {} selectElement: $select, minLength: 3, showAllValues: $select.multiple, - showNoOptionsFound: true, - onConfirm: function (query) { - let matchingOption - if (query) { - matchingOption = [].filter.call($select.options, function (option) { - return (option.textContent || option.innerText) === query - })[0] - } else { - matchingOption = [].filter.call($select.options, function (option) { - return option.value === '' - })[0] - } - if (matchingOption) { - matchingOption.selected = true - } - } + showNoOptionsFound: true } const assignedOptions = JSON.parse( From 3fedc51682bc6ec810c4b7cb5b645e8fc906832b Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Mon, 17 Feb 2025 16:02:00 +0000 Subject: [PATCH 03/17] Add 'lint:prettier:fix' script for applying Prettier It's not immediately clear how to auto-fix any issues detected by Prettier, nor is there an obvious way of getting Prettier to even tell you what the issues _are_. So adding a script that auto-fixes, which the developer can run manually if needed. --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index bf90ea49a09..94b7adb0f8c 100644 --- a/package.json +++ b/package.json @@ -9,6 +9,7 @@ "lint:js": "eslint --cache --cache-location .cache/eslint --color --ignore-path .gitignore -- \"**/*.js\"", "lint:scss": "stylelint app/assets/stylesheets/", "lint:prettier": "prettier --cache --cache-location .cache/prettier --cache-strategy content --check -- \"**/*.{js,scss}\"", + "lint:prettier:fix": "prettier --write \"**/*.{js,scss}\"", "jasmine:prepare": "RAILS_ENV=test bundle exec rails assets:clobber assets:precompile", "jasmine:ci": "yarn run jasmine:prepare && yarn run jasmine-browser-runner runSpecs", "jasmine:browser": "yarn run jasmine:prepare && yarn run jasmine-browser-runner" From 53f6f6804a4d4b734929b706215c8128d2d74af9 Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Mon, 17 Feb 2025 16:44:33 +0000 Subject: [PATCH 04/17] Add new 'include_blank' option for autocomplete module It's a common use case to show a blank option on the select dropdown, and it feels wrong that in each case where that's required, we have to prefix options with `[""]`. It should just be a natively supported feature (allowing us to change the underlying implementation much more easily). This commit carries over the `include_blank` option from the 'select_with_search' component so that we have feature parity between the two components. It also uses the new option on existing 'autocomplete' instances. --- .../admin/detailed_guides/_form.html.erb | 3 ++- .../editions/_appointment_fields.html.erb | 3 ++- .../editions/_organisation_fields.html.erb | 3 ++- .../_statistical_data_set_fields.html.erb | 3 ++- .../editions/_topical_event_fields.html.erb | 3 ++- .../_worldwide_organisation_fields.html.erb | 3 ++- app/views/admin/needs/edit.html.erb | 3 ++- .../organisations/_closed_fields.html.erb | 14 ++++++------- app/views/admin/organisations/_form.html.erb | 13 ++++++------ app/views/admin/users/edit.html.erb | 3 ++- app/views/components/_autocomplete.html.erb | 6 +++++- app/views/components/docs/autocomplete.yml | 21 ++++++++++++++++--- .../content_block/shared/_form.html.erb | 3 ++- test/components/autocomplete_test.rb | 8 ++++++- 14 files changed, 61 insertions(+), 28 deletions(-) diff --git a/app/views/admin/detailed_guides/_form.html.erb b/app/views/admin/detailed_guides/_form.html.erb index 8f7a5ba0ce4..b1e593e4bd2 100644 --- a/app/views/admin/detailed_guides/_form.html.erb +++ b/app/views/admin/detailed_guides/_form.html.erb @@ -19,12 +19,13 @@ id: "edition_related_detailed_guide_ids", name: "edition[related_detailed_guide_ids][]", error_items: errors_for(edition.errors, :related_detailed_guide_ids), + include_blank: true, label: { text: "Related guides", heading_size: "m", }, select: { - options: [""] + taggable_detailed_guides_container, + options: taggable_detailed_guides_container, multiple: true, selected: edition.related_detailed_guide_ids, }, diff --git a/app/views/admin/editions/_appointment_fields.html.erb b/app/views/admin/editions/_appointment_fields.html.erb index a0092681406..392928b458c 100644 --- a/app/views/admin/editions/_appointment_fields.html.erb +++ b/app/views/admin/editions/_appointment_fields.html.erb @@ -2,12 +2,13 @@ <%= render "components/autocomplete", { id: "edition_role_appointment_ids", name: "edition[role_appointment_ids][]", + include_blank: true, label: { text: "Ministers", heading_size: "m", }, select: { - options: [""] + taggable_ministerial_role_appointments_container, + options: taggable_ministerial_role_appointments_container, multiple: true, selected: edition.role_appointment_ids, }, diff --git a/app/views/admin/editions/_organisation_fields.html.erb b/app/views/admin/editions/_organisation_fields.html.erb index 9479994c4c3..bcd46e91418 100644 --- a/app/views/admin/editions/_organisation_fields.html.erb +++ b/app/views/admin/editions/_organisation_fields.html.erb @@ -32,6 +32,7 @@ <%= render "components/autocomplete", { id: "edition_supporting_organisation_ids", name: "edition[supporting_organisation_ids][]", + include_blank: true, label: { text: "Supporting organisations", heading_size: "m", @@ -39,7 +40,7 @@ select: { multiple: true, selected: edition.edition_organisations.reject(&:lead?).map(&:organisation_id), - options: [""] + taggable_organisations_container, + options: taggable_organisations_container, }, } %> <% end %> diff --git a/app/views/admin/editions/_statistical_data_set_fields.html.erb b/app/views/admin/editions/_statistical_data_set_fields.html.erb index 005c515efe7..e5ede8f11b6 100644 --- a/app/views/admin/editions/_statistical_data_set_fields.html.erb +++ b/app/views/admin/editions/_statistical_data_set_fields.html.erb @@ -2,12 +2,13 @@ <%= render "components/autocomplete", { id: "edition_statistical_data_set_document_ids", name: "edition[statistical_data_set_document_ids][]", + include_blank: true, label: { text: "Statistical data sets", heading_size: "m", }, select: { - options: [""] + taggable_statistical_data_sets_container, + options: taggable_statistical_data_sets_container, multiple: true, selected: edition.statistical_data_set_document_ids, }, diff --git a/app/views/admin/editions/_topical_event_fields.html.erb b/app/views/admin/editions/_topical_event_fields.html.erb index 5c8715e6287..b4f9e171ee9 100644 --- a/app/views/admin/editions/_topical_event_fields.html.erb +++ b/app/views/admin/editions/_topical_event_fields.html.erb @@ -2,12 +2,13 @@ <%= render "components/autocomplete", { id: "edition_topical_event_ids", name: "edition[topical_event_ids][]", + include_blank: true, label: { text: "Topical events", heading_size: "m", }, select: { - options: [""] + TopicalEvent.order(:name).map { |topical_event| [topical_event.name, topical_event.id]}, + options: TopicalEvent.order(:name).map { |topical_event| [topical_event.name, topical_event.id]}, multiple: true, selected: edition.topical_event_ids, }, diff --git a/app/views/admin/editions/_worldwide_organisation_fields.html.erb b/app/views/admin/editions/_worldwide_organisation_fields.html.erb index abc282edc76..ae5a7f7fd8d 100644 --- a/app/views/admin/editions/_worldwide_organisation_fields.html.erb +++ b/app/views/admin/editions/_worldwide_organisation_fields.html.erb @@ -5,12 +5,13 @@ id: "edition_worldwide_organisation_document_ids", name: "edition[worldwide_organisation_document_ids][]", error_items: errors_for(edition.errors, :worldwide_organisations), + include_blank: true, label: { text: "Worldwide organisations" + "#{' (required)' if required}", heading_size: "m", }, select: { - options: [""] + taggable_worldwide_organisations_container, + options: taggable_worldwide_organisations_container, multiple: true, selected: edition.worldwide_organisation_document_ids, }, diff --git a/app/views/admin/needs/edit.html.erb b/app/views/admin/needs/edit.html.erb index e8927bc2ad2..df6b28fd159 100644 --- a/app/views/admin/needs/edit.html.erb +++ b/app/views/admin/needs/edit.html.erb @@ -8,6 +8,7 @@ <%= render "components/autocomplete", { id: "need_ids", name: "need_ids[]", + include_blank: true, label: { text: "Select associated user needs", bold: true, @@ -15,7 +16,7 @@ }, heading_size: "l", select: { - options: [""] + taggable_needs_container, + options: taggable_needs_container, multiple: true, selected: @document.need_ids, }, diff --git a/app/views/admin/organisations/_closed_fields.html.erb b/app/views/admin/organisations/_closed_fields.html.erb index 6b70dd52b1d..243968945f7 100644 --- a/app/views/admin/organisations/_closed_fields.html.erb +++ b/app/views/admin/organisations/_closed_fields.html.erb @@ -47,18 +47,18 @@ <%= render "components/autocomplete", { id: "organisation_superseding_organisation_ids", name: "organisation[superseding_organisation_ids][]", + include_blank: true, label: { text: "Superseding organisations", heading_size: "m", }, select: { - options: [""] + - (Organisation.with_translations(:en) - [organisation]).map do |org| - [ - org.name, - org.id, - ] - end, + options: (Organisation.with_translations(:en) - [organisation]).map do |org| + [ + org.name, + org.id, + ] + end, multiple: true, selected: organisation.superseding_organisation_ids, }, diff --git a/app/views/admin/organisations/_form.html.erb b/app/views/admin/organisations/_form.html.erb index c1676e692e3..b907befd63f 100644 --- a/app/views/admin/organisations/_form.html.erb +++ b/app/views/admin/organisations/_form.html.erb @@ -246,13 +246,12 @@ heading_size: "m", }, select: { - options: [""] + - (Organisation.with_translations(:en) - [organisation]).map do |org| - [ - org.name, - org.id, - ] - end, + options: (Organisation.with_translations(:en) - [organisation]).map do |org| + [ + org.name, + org.id, + ] + end, multiple: true, selected: organisation.parent_organisation_ids, }, diff --git a/app/views/admin/users/edit.html.erb b/app/views/admin/users/edit.html.erb index e3d0648466d..d97a83f466e 100644 --- a/app/views/admin/users/edit.html.erb +++ b/app/views/admin/users/edit.html.erb @@ -8,6 +8,7 @@ <%= render "components/autocomplete", { id: "user_world_location_ids", name: "user[world_location_ids][]", + include_blank: true, label: { text: "Locations", heading_size: "l", @@ -15,7 +16,7 @@ select: { multiple: true, selected: form.object.world_location_ids, - options: [""] + WorldLocation.all.map { |l| [l.name, l.id] }, + options: WorldLocation.all.map { |l| [l.name, l.id] }, }, } %>
diff --git a/app/views/components/_autocomplete.html.erb b/app/views/components/_autocomplete.html.erb index bbbb02fb851..3ce49286357 100644 --- a/app/views/components/_autocomplete.html.erb +++ b/app/views/components/_autocomplete.html.erb @@ -4,6 +4,7 @@ error_items ||= [] aria = error_id if error_items.any? select ||= {} + include_blank ||= false autocomplete_configuration_options ||= { showAllValues: true, } @@ -38,7 +39,10 @@ <% end %> <%= select_tag name, - options_for_select(select[:options], select[:selected]), + options_for_select( + include_blank ? ([""] + select[:options]) : select[:options], + select[:selected], + ), id: id, class: "govuk-select", size: select[:size], diff --git a/app/views/components/docs/autocomplete.yml b/app/views/components/docs/autocomplete.yml index c8bc159db00..c4cddcf54e7 100644 --- a/app/views/components/docs/autocomplete.yml +++ b/app/views/components/docs/autocomplete.yml @@ -17,7 +17,24 @@ examples: select: options: - - - + - France + - fr + - + - Germany + - de + - + - United Kingdom + - uk + + with_blank_option: + data: + id: autocomplete + name: autocomplete + include_blank: true + label: + text: Select your country + select: + options: - - France - fr @@ -101,8 +118,6 @@ examples: text: Select your country select: options: - - - - - - France - fr diff --git a/lib/engines/content_block_manager/app/views/content_block_manager/content_block/shared/_form.html.erb b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/shared/_form.html.erb index 8daa338cbf2..efb69556efa 100644 --- a/lib/engines/content_block_manager/app/views/content_block_manager/content_block/shared/_form.html.erb +++ b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/shared/_form.html.erb @@ -30,12 +30,13 @@ id: "#{parent_class}_lead_organisation", name: "content_block/edition[organisation_id]", error_items: errors_for(@form.content_block_edition.errors, "lead_organisation".to_sym), + include_blank: true, label: { text: "Lead organisation", }, select: { multiple: false, - options: [""] + taggable_organisations_container, + options: taggable_organisations_container, selected: @form.content_block_edition.edition_organisation&.organisation_id, }, } %> diff --git a/test/components/autocomplete_test.rb b/test/components/autocomplete_test.rb index 2b7fe717e1c..007f6fb495c 100644 --- a/test/components/autocomplete_test.rb +++ b/test/components/autocomplete_test.rb @@ -14,7 +14,6 @@ def component_data }, select: { options: [ - [""], ["France", "fr"], ["Germany", "de"], ["United Kingdom", "uk"], @@ -43,6 +42,13 @@ def component_data assert_select ".app-c-autocomplete .govuk-select option[value='de'][selected='selected']" end + test "renders with a blank option" do + data = component_data + data[:include_blank] = true + render_component(data) + assert_select ".app-c-autocomplete .govuk-select option[value='']" + end + test "renders with an error" do data = component_data data[:error_items] = [ From 0eeb1601dc5fb42f78f1be0b6a355731292f7654 Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Tue, 18 Feb 2025 14:23:12 +0000 Subject: [PATCH 05/17] Remove 'bold' and 'required' label booleans from Needs form "Needs" is a Maslow thing and is going away. This is the only occurrence of those properties in all 'autocomplete' calls and so removing them will simplify delivering feature parity with the 'select with search' component. The [current implementation](https://github.com/alphagov/whitehall/blob/20fcfed3948f218d05b932ee910f91161e245c8f/app/views/components/_autocomplete.html.erb#L23-L25) of autocomplete just merges these properties into a hash that is passed to the 'label' component. Whilst label supports 'bold', it doesn't appear to support 'required', so it's unlikely that was achieving anything: https://components.publishing.service.gov.uk/component-guide/label --- app/views/admin/needs/edit.html.erb | 2 -- app/views/components/docs/autocomplete.yml | 1 - 2 files changed, 3 deletions(-) diff --git a/app/views/admin/needs/edit.html.erb b/app/views/admin/needs/edit.html.erb index df6b28fd159..2b2666b23e8 100644 --- a/app/views/admin/needs/edit.html.erb +++ b/app/views/admin/needs/edit.html.erb @@ -11,8 +11,6 @@ include_blank: true, label: { text: "Select associated user needs", - bold: true, - required: true, }, heading_size: "l", select: { diff --git a/app/views/components/docs/autocomplete.yml b/app/views/components/docs/autocomplete.yml index c4cddcf54e7..3b528be8213 100644 --- a/app/views/components/docs/autocomplete.yml +++ b/app/views/components/docs/autocomplete.yml @@ -51,7 +51,6 @@ examples: name: autocomplete-selected label: text: Select your country - bold: true hint: Only a few countries are available select: options: From 9a496daf8a690419949d5aee5e3be946f09318f4 Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Tue, 18 Feb 2025 16:26:07 +0000 Subject: [PATCH 06/17] Remove unused 'size' property from 'autocomplete' select This didn't appear to be in use anywhere. --- app/views/components/_autocomplete.html.erb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/views/components/_autocomplete.html.erb b/app/views/components/_autocomplete.html.erb index 3ce49286357..883f80bc799 100644 --- a/app/views/components/_autocomplete.html.erb +++ b/app/views/components/_autocomplete.html.erb @@ -45,6 +45,5 @@ ), id: id, class: "govuk-select", - size: select[:size], multiple: select[:multiple] %> <% end %> From f641b81b1489d4d4d85aa127532513e03db1a40a Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Tue, 18 Feb 2025 14:33:41 +0000 Subject: [PATCH 07/17] Move 'heading_size' out of 'label' property and into root hash This brings the 'autocomplete' config closer in line with the 'select with search' config, which defines 'heading_size' in the root hash rather than in the `label`. It appears govuk_publishing_components is equally divided, so there is no strong reason to go with one approach vs the other: https://github.com/alphagov/govuk_publishing_components/issues/4641 Note that the 'select_with_search' component makes use of the `GovukPublishingComponents::Presenters::SharedHelper` helper to determine whether the passed 'heading_size' is valid. I did add a test to 'autocomplete' with a view to adding that validation to 'autocomplete', but the test passed without modification, so it appears the component already avoids using invalid values and thus there's no need for us to check for it in Whitehall: ``` test "avoids passing heading size to label component if invalid value" do data = component_data data[:heading_size] = "sausages" assert_select ".govuk-label.govuk-label--sausages", { count: 0 } assert_select ".govuk-label", { count: 1 } end ``` --- .../admin/detailed_guides/_form.html.erb | 2 +- .../editions/_appointment_fields.html.erb | 2 +- .../editions/_organisation_fields.html.erb | 2 +- .../admin/editions/_role_fields.html.erb | 2 +- .../_statistical_data_set_fields.html.erb | 2 +- .../editions/_topical_event_fields.html.erb | 2 +- .../editions/_world_location_fields.html.erb | 2 +- .../_worldwide_organisation_fields.html.erb | 2 +- .../admin/historical_accounts/_form.html.erb | 2 +- .../organisations/_closed_fields.html.erb | 2 +- app/views/admin/organisations/_form.html.erb | 2 +- app/views/admin/roles/_form.html.erb | 2 +- .../statistics_announcements/_form.html.erb | 2 +- app/views/admin/users/edit.html.erb | 2 +- app/views/components/_autocomplete.html.erb | 2 ++ app/views/components/docs/autocomplete.yml | 20 +++++++++++++++++++ test/components/autocomplete_test.rb | 7 +++++++ 17 files changed, 43 insertions(+), 14 deletions(-) diff --git a/app/views/admin/detailed_guides/_form.html.erb b/app/views/admin/detailed_guides/_form.html.erb index b1e593e4bd2..1b0fd7c38ad 100644 --- a/app/views/admin/detailed_guides/_form.html.erb +++ b/app/views/admin/detailed_guides/_form.html.erb @@ -22,8 +22,8 @@ include_blank: true, label: { text: "Related guides", - heading_size: "m", }, + heading_size: "m", select: { options: taggable_detailed_guides_container, multiple: true, diff --git a/app/views/admin/editions/_appointment_fields.html.erb b/app/views/admin/editions/_appointment_fields.html.erb index 392928b458c..64067d95405 100644 --- a/app/views/admin/editions/_appointment_fields.html.erb +++ b/app/views/admin/editions/_appointment_fields.html.erb @@ -5,8 +5,8 @@ include_blank: true, label: { text: "Ministers", - heading_size: "m", }, + heading_size: "m", select: { options: taggable_ministerial_role_appointments_container, multiple: true, diff --git a/app/views/admin/editions/_organisation_fields.html.erb b/app/views/admin/editions/_organisation_fields.html.erb index bcd46e91418..6989a9f301f 100644 --- a/app/views/admin/editions/_organisation_fields.html.erb +++ b/app/views/admin/editions/_organisation_fields.html.erb @@ -35,8 +35,8 @@ include_blank: true, label: { text: "Supporting organisations", - heading_size: "m", }, + heading_size: "m", select: { multiple: true, selected: edition.edition_organisations.reject(&:lead?).map(&:organisation_id), diff --git a/app/views/admin/editions/_role_fields.html.erb b/app/views/admin/editions/_role_fields.html.erb index 9ad4b96ca2b..2d83b35497a 100644 --- a/app/views/admin/editions/_role_fields.html.erb +++ b/app/views/admin/editions/_role_fields.html.erb @@ -7,8 +7,8 @@ error_items: errors_for(edition.errors, :roles), label: { text: "Roles" + "#{' (required)' if required}", - heading_size: "m", }, + heading_size: "m", select: { options: taggable_roles_container, multiple: true, diff --git a/app/views/admin/editions/_statistical_data_set_fields.html.erb b/app/views/admin/editions/_statistical_data_set_fields.html.erb index e5ede8f11b6..e0c049c72a2 100644 --- a/app/views/admin/editions/_statistical_data_set_fields.html.erb +++ b/app/views/admin/editions/_statistical_data_set_fields.html.erb @@ -5,8 +5,8 @@ include_blank: true, label: { text: "Statistical data sets", - heading_size: "m", }, + heading_size: "m", select: { options: taggable_statistical_data_sets_container, multiple: true, diff --git a/app/views/admin/editions/_topical_event_fields.html.erb b/app/views/admin/editions/_topical_event_fields.html.erb index b4f9e171ee9..cceb5c2820d 100644 --- a/app/views/admin/editions/_topical_event_fields.html.erb +++ b/app/views/admin/editions/_topical_event_fields.html.erb @@ -5,8 +5,8 @@ include_blank: true, label: { text: "Topical events", - heading_size: "m", }, + heading_size: "m", select: { options: TopicalEvent.order(:name).map { |topical_event| [topical_event.name, topical_event.id]}, multiple: true, diff --git a/app/views/admin/editions/_world_location_fields.html.erb b/app/views/admin/editions/_world_location_fields.html.erb index 5028deab10e..be315e93d4d 100644 --- a/app/views/admin/editions/_world_location_fields.html.erb +++ b/app/views/admin/editions/_world_location_fields.html.erb @@ -7,8 +7,8 @@ error_items: errors_for(edition.errors, :world_locations), label: { text: "World locations" + "#{' (required)' if required}", - heading_size: "m", }, + heading_size: "m", select: { options: taggable_world_locations_container, multiple: true, diff --git a/app/views/admin/editions/_worldwide_organisation_fields.html.erb b/app/views/admin/editions/_worldwide_organisation_fields.html.erb index ae5a7f7fd8d..1fd07ec4b30 100644 --- a/app/views/admin/editions/_worldwide_organisation_fields.html.erb +++ b/app/views/admin/editions/_worldwide_organisation_fields.html.erb @@ -8,8 +8,8 @@ include_blank: true, label: { text: "Worldwide organisations" + "#{' (required)' if required}", - heading_size: "m", }, + heading_size: "m", select: { options: taggable_worldwide_organisations_container, multiple: true, diff --git a/app/views/admin/historical_accounts/_form.html.erb b/app/views/admin/historical_accounts/_form.html.erb index 79166abc9f0..aafa7189c66 100644 --- a/app/views/admin/historical_accounts/_form.html.erb +++ b/app/views/admin/historical_accounts/_form.html.erb @@ -22,8 +22,8 @@ error_items: errors_for(historical_account.errors, :political_parties), label: { text: "Political parties (required)", - heading_size: "l", }, + heading_size: "l", select: { options: PoliticalParty.all.map do |party| [ diff --git a/app/views/admin/organisations/_closed_fields.html.erb b/app/views/admin/organisations/_closed_fields.html.erb index 243968945f7..0408d80bcb4 100644 --- a/app/views/admin/organisations/_closed_fields.html.erb +++ b/app/views/admin/organisations/_closed_fields.html.erb @@ -50,8 +50,8 @@ include_blank: true, label: { text: "Superseding organisations", - heading_size: "m", }, + heading_size: "m", select: { options: (Organisation.with_translations(:en) - [organisation]).map do |org| [ diff --git a/app/views/admin/organisations/_form.html.erb b/app/views/admin/organisations/_form.html.erb index b907befd63f..162b64cb4bf 100644 --- a/app/views/admin/organisations/_form.html.erb +++ b/app/views/admin/organisations/_form.html.erb @@ -243,8 +243,8 @@ name: "organisation[parent_organisation_ids][]", label: { text: "Sponsoring organisations", - heading_size: "m", }, + heading_size: "m", select: { options: (Organisation.with_translations(:en) - [organisation]).map do |org| [ diff --git a/app/views/admin/roles/_form.html.erb b/app/views/admin/roles/_form.html.erb index 1add094a7f3..6520b244c67 100644 --- a/app/views/admin/roles/_form.html.erb +++ b/app/views/admin/roles/_form.html.erb @@ -33,8 +33,8 @@ id: "role_organisation_ids", label: { text: "Organisations", - heading_size: "l", }, + heading_size: "l", name: "role[organisation_ids][]", select: { options: options_from_collection_for_select(Organisation.with_translations(:en), "id", "select_name", role.organisation_ids), diff --git a/app/views/admin/statistics_announcements/_form.html.erb b/app/views/admin/statistics_announcements/_form.html.erb index 031dc992a45..8768ee40aa3 100644 --- a/app/views/admin/statistics_announcements/_form.html.erb +++ b/app/views/admin/statistics_announcements/_form.html.erb @@ -55,8 +55,8 @@ id: "statistics_announcement_organisations", label: { text: "Organisations (required)", - heading_size: "l", }, + heading_size: "l", name: "statistics_announcement[organisation_ids][]", select: { options: options_from_collection_for_select(Organisation.with_translations.order("organisation_translations.name"), "id", "name", statistics_announcement.organisation_ids), diff --git a/app/views/admin/users/edit.html.erb b/app/views/admin/users/edit.html.erb index d97a83f466e..85da202147c 100644 --- a/app/views/admin/users/edit.html.erb +++ b/app/views/admin/users/edit.html.erb @@ -11,8 +11,8 @@ include_blank: true, label: { text: "Locations", - heading_size: "l", }, + heading_size: "l", select: { multiple: true, selected: form.object.world_location_ids, diff --git a/app/views/components/_autocomplete.html.erb b/app/views/components/_autocomplete.html.erb index 883f80bc799..fb5fb6fff37 100644 --- a/app/views/components/_autocomplete.html.erb +++ b/app/views/components/_autocomplete.html.erb @@ -5,6 +5,7 @@ aria = error_id if error_items.any? select ||= {} include_blank ||= false + heading_size ||= nil autocomplete_configuration_options ||= { showAllValues: true, } @@ -22,6 +23,7 @@ <%= tag.div class: root_classes, data: data_attributes do %> <%= render "govuk_publishing_components/components/label", { html_for: id, + heading_size:, }.merge(label.symbolize_keys) %> <% if error_items.any? %> diff --git a/app/views/components/docs/autocomplete.yml b/app/views/components/docs/autocomplete.yml index 3b528be8213..19491ccd433 100644 --- a/app/views/components/docs/autocomplete.yml +++ b/app/views/components/docs/autocomplete.yml @@ -45,6 +45,26 @@ examples: - United Kingdom - uk + with_custom_heading_size: + data: + id: autocomplete + name: autocomplete + include_blank: true + label: + text: Select your country + heading_size: xl + select: + options: + - + - France + - fr + - + - Germany + - de + - + - United Kingdom + - uk + with_selected_value: data: id: autocomplete-selected diff --git a/test/components/autocomplete_test.rb b/test/components/autocomplete_test.rb index 007f6fb495c..c362f750c0a 100644 --- a/test/components/autocomplete_test.rb +++ b/test/components/autocomplete_test.rb @@ -49,6 +49,13 @@ def component_data assert_select ".app-c-autocomplete .govuk-select option[value='']" end + test "passes heading size to label component" do + data = component_data + data[:heading_size] = "xl" + render_component(data) + assert_select ".govuk-label.govuk-label--xl" + end + test "renders with an error" do data = component_data data[:error_items] = [ From f718e8f465756dbaec72d069dc06027f53226e78 Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Thu, 20 Feb 2025 10:24:51 +0000 Subject: [PATCH 08/17] Remove unused `is_page_heading` property This is a case of YAGNI. It's supported by the 'select' component (which uses it in one place in Whitehall: https://github.com/alphagov/whitehall/blob/514ba658e64b3dd3b5bb12a607e47f3f0d80422c/app/views/admin/worldwide_organisations_main_offices/show.erb#L32) but it isn't used by any select_with_search component. We can always add it back in later if we need it. Removing it simplifies the proposition between 'autocomplete' and 'select with search' (i.e. we don't need to port over the `is_page_heading` behaviour to the 'autocomplete' component if we were to decide to switch over to that component exclusively). --- app/views/components/_select_with_search.html.erb | 7 +------ app/views/components/docs/select_with_search.yml | 12 ------------ 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/app/views/components/_select_with_search.html.erb b/app/views/components/_select_with_search.html.erb index cd522555961..f3e98901141 100644 --- a/app/views/components/_select_with_search.html.erb +++ b/app/views/components/_select_with_search.html.erb @@ -2,7 +2,6 @@ id ||= false label ||= false name ||= id - is_page_heading ||= false shared_helper = GovukPublishingComponents::Presenters::SharedHelper.new(local_assigns) heading_size = false unless shared_helper.valid_heading_size?(heading_size) @@ -12,11 +11,7 @@ %> <%= content_tag :div, class: select_helper.css_classes, data: select_helper.data_attributes do %> - <% if is_page_heading %> - <%= tag.h1 label_tag(id, label, class: select_helper.label_classes, id: "#{id}_label"), class: "govuk-heading-xl" %> - <% else %> - <%= label_tag(id, label, class: select_helper.label_classes, id: "#{id}_label") %> - <% end %> + <%= label_tag(id, label, class: select_helper.label_classes, id: "#{id}_label") %> <% if select_helper.hint %> <%= render "govuk_publishing_components/components/hint", { diff --git a/app/views/components/docs/select_with_search.yml b/app/views/components/docs/select_with_search.yml index bd9f6feed93..6b466965419 100644 --- a/app/views/components/docs/select_with_search.yml +++ b/app/views/components/docs/select_with_search.yml @@ -170,15 +170,3 @@ examples: value: option2 - text: Option three value: option3 - with_page_heading: - description: This adds a `h1` element with a label element inside containing the text supplied. - data: - id: select-with-page-heading - label: This is a page heading - heading_size: xl - is_page_heading: true - options: - - text: Option one - value: option1 - - text: Option two - value: option2 From 490c1eb3e610665d55d2d8dc3577aaec4e7cce4a Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Tue, 18 Feb 2025 14:45:01 +0000 Subject: [PATCH 09/17] Flatten 'label' property There is no clear convention in govuk_publishing_components as to whether a label should be a string or a hash. Components with string labels: - https://components.publishing.service.gov.uk/component-guide/copy_to_clipboard - https://components.publishing.service.gov.uk/component-guide/password_input - https://components.publishing.service.gov.uk/component-guide/select - https://components.publishing.service.gov.uk/component-guide/search_with_autocomplete Components with hash labels: - https://components.publishing.service.gov.uk/component-guide/file_upload - https://components.publishing.service.gov.uk/component-guide/character_count - https://components.publishing.service.gov.uk/component-guide/input Given we only now have a 'text' subproperty of 'label' for the 'autocomplete' component, the hash is a bit clunky to use and offers no real benefit. So bringing it in line with 'select with search', we're switching to a string, which will make it easier to consolidate the components. --- .../admin/detailed_guides/_form.html.erb | 4 +--- .../editions/_appointment_fields.html.erb | 4 +--- .../editions/_organisation_fields.html.erb | 4 +--- .../admin/editions/_role_fields.html.erb | 4 +--- .../_statistical_data_set_fields.html.erb | 4 +--- .../editions/_topical_event_fields.html.erb | 4 +--- .../editions/_world_location_fields.html.erb | 4 +--- .../_worldwide_organisation_fields.html.erb | 4 +--- .../admin/historical_accounts/_form.html.erb | 4 +--- app/views/admin/needs/edit.html.erb | 4 +--- .../organisations/_closed_fields.html.erb | 4 +--- app/views/admin/organisations/_form.html.erb | 4 +--- app/views/admin/roles/_form.html.erb | 4 +--- .../statistics_announcements/_form.html.erb | 4 +--- app/views/admin/users/edit.html.erb | 4 +--- app/views/components/_autocomplete.html.erb | 3 ++- app/views/components/docs/autocomplete.yml | 24 +++++++------------ .../content_block/shared/_form.html.erb | 4 +--- test/components/autocomplete_test.rb | 4 +--- 19 files changed, 27 insertions(+), 68 deletions(-) diff --git a/app/views/admin/detailed_guides/_form.html.erb b/app/views/admin/detailed_guides/_form.html.erb index 1b0fd7c38ad..d770dd0c544 100644 --- a/app/views/admin/detailed_guides/_form.html.erb +++ b/app/views/admin/detailed_guides/_form.html.erb @@ -20,9 +20,7 @@ name: "edition[related_detailed_guide_ids][]", error_items: errors_for(edition.errors, :related_detailed_guide_ids), include_blank: true, - label: { - text: "Related guides", - }, + label: "Related guides", heading_size: "m", select: { options: taggable_detailed_guides_container, diff --git a/app/views/admin/editions/_appointment_fields.html.erb b/app/views/admin/editions/_appointment_fields.html.erb index 64067d95405..8e0c360fc16 100644 --- a/app/views/admin/editions/_appointment_fields.html.erb +++ b/app/views/admin/editions/_appointment_fields.html.erb @@ -3,9 +3,7 @@ id: "edition_role_appointment_ids", name: "edition[role_appointment_ids][]", include_blank: true, - label: { - text: "Ministers", - }, + label: "Ministers", heading_size: "m", select: { options: taggable_ministerial_role_appointments_container, diff --git a/app/views/admin/editions/_organisation_fields.html.erb b/app/views/admin/editions/_organisation_fields.html.erb index 6989a9f301f..7637a2fc8db 100644 --- a/app/views/admin/editions/_organisation_fields.html.erb +++ b/app/views/admin/editions/_organisation_fields.html.erb @@ -33,9 +33,7 @@ id: "edition_supporting_organisation_ids", name: "edition[supporting_organisation_ids][]", include_blank: true, - label: { - text: "Supporting organisations", - }, + label: "Supporting organisations", heading_size: "m", select: { multiple: true, diff --git a/app/views/admin/editions/_role_fields.html.erb b/app/views/admin/editions/_role_fields.html.erb index 2d83b35497a..a5f13e96f98 100644 --- a/app/views/admin/editions/_role_fields.html.erb +++ b/app/views/admin/editions/_role_fields.html.erb @@ -5,9 +5,7 @@ id: "edition_roles", name: "edition[role_ids][]", error_items: errors_for(edition.errors, :roles), - label: { - text: "Roles" + "#{' (required)' if required}", - }, + label: "Roles" + "#{' (required)' if required}", heading_size: "m", select: { options: taggable_roles_container, diff --git a/app/views/admin/editions/_statistical_data_set_fields.html.erb b/app/views/admin/editions/_statistical_data_set_fields.html.erb index e0c049c72a2..d640caa2be5 100644 --- a/app/views/admin/editions/_statistical_data_set_fields.html.erb +++ b/app/views/admin/editions/_statistical_data_set_fields.html.erb @@ -3,9 +3,7 @@ id: "edition_statistical_data_set_document_ids", name: "edition[statistical_data_set_document_ids][]", include_blank: true, - label: { - text: "Statistical data sets", - }, + label: "Statistical data sets", heading_size: "m", select: { options: taggable_statistical_data_sets_container, diff --git a/app/views/admin/editions/_topical_event_fields.html.erb b/app/views/admin/editions/_topical_event_fields.html.erb index cceb5c2820d..57d47ec0edd 100644 --- a/app/views/admin/editions/_topical_event_fields.html.erb +++ b/app/views/admin/editions/_topical_event_fields.html.erb @@ -3,9 +3,7 @@ id: "edition_topical_event_ids", name: "edition[topical_event_ids][]", include_blank: true, - label: { - text: "Topical events", - }, + label: "Topical events", heading_size: "m", select: { options: TopicalEvent.order(:name).map { |topical_event| [topical_event.name, topical_event.id]}, diff --git a/app/views/admin/editions/_world_location_fields.html.erb b/app/views/admin/editions/_world_location_fields.html.erb index be315e93d4d..18ae572ed35 100644 --- a/app/views/admin/editions/_world_location_fields.html.erb +++ b/app/views/admin/editions/_world_location_fields.html.erb @@ -5,9 +5,7 @@ id: "edition_world_locations", name: "edition[world_location_ids][]", error_items: errors_for(edition.errors, :world_locations), - label: { - text: "World locations" + "#{' (required)' if required}", - }, + label: "World locations" + "#{' (required)' if required}", heading_size: "m", select: { options: taggable_world_locations_container, diff --git a/app/views/admin/editions/_worldwide_organisation_fields.html.erb b/app/views/admin/editions/_worldwide_organisation_fields.html.erb index 1fd07ec4b30..8f9b24672b5 100644 --- a/app/views/admin/editions/_worldwide_organisation_fields.html.erb +++ b/app/views/admin/editions/_worldwide_organisation_fields.html.erb @@ -6,9 +6,7 @@ name: "edition[worldwide_organisation_document_ids][]", error_items: errors_for(edition.errors, :worldwide_organisations), include_blank: true, - label: { - text: "Worldwide organisations" + "#{' (required)' if required}", - }, + label: "Worldwide organisations" + "#{' (required)' if required}", heading_size: "m", select: { options: taggable_worldwide_organisations_container, diff --git a/app/views/admin/historical_accounts/_form.html.erb b/app/views/admin/historical_accounts/_form.html.erb index aafa7189c66..f6e8e1b4820 100644 --- a/app/views/admin/historical_accounts/_form.html.erb +++ b/app/views/admin/historical_accounts/_form.html.erb @@ -20,9 +20,7 @@ id: "historical_account_political_parties", name: "historical_account[political_party_ids][]", error_items: errors_for(historical_account.errors, :political_parties), - label: { - text: "Political parties (required)", - }, + label: "Political parties (required)", heading_size: "l", select: { options: PoliticalParty.all.map do |party| diff --git a/app/views/admin/needs/edit.html.erb b/app/views/admin/needs/edit.html.erb index 2b2666b23e8..cee326271ec 100644 --- a/app/views/admin/needs/edit.html.erb +++ b/app/views/admin/needs/edit.html.erb @@ -9,9 +9,7 @@ id: "need_ids", name: "need_ids[]", include_blank: true, - label: { - text: "Select associated user needs", - }, + label: "Select associated user needs", heading_size: "l", select: { options: taggable_needs_container, diff --git a/app/views/admin/organisations/_closed_fields.html.erb b/app/views/admin/organisations/_closed_fields.html.erb index 0408d80bcb4..a9889270362 100644 --- a/app/views/admin/organisations/_closed_fields.html.erb +++ b/app/views/admin/organisations/_closed_fields.html.erb @@ -48,9 +48,7 @@ id: "organisation_superseding_organisation_ids", name: "organisation[superseding_organisation_ids][]", include_blank: true, - label: { - text: "Superseding organisations", - }, + label: "Superseding organisations", heading_size: "m", select: { options: (Organisation.with_translations(:en) - [organisation]).map do |org| diff --git a/app/views/admin/organisations/_form.html.erb b/app/views/admin/organisations/_form.html.erb index 162b64cb4bf..94ff4374790 100644 --- a/app/views/admin/organisations/_form.html.erb +++ b/app/views/admin/organisations/_form.html.erb @@ -241,9 +241,7 @@ <%= render "components/autocomplete", { id: "organisation_parent_organisation_ids", name: "organisation[parent_organisation_ids][]", - label: { - text: "Sponsoring organisations", - }, + label: "Sponsoring organisations", heading_size: "m", select: { options: (Organisation.with_translations(:en) - [organisation]).map do |org| diff --git a/app/views/admin/roles/_form.html.erb b/app/views/admin/roles/_form.html.erb index 6520b244c67..5809fb286d2 100644 --- a/app/views/admin/roles/_form.html.erb +++ b/app/views/admin/roles/_form.html.erb @@ -31,9 +31,7 @@ <%= render "components/autocomplete", { id: "role_organisation_ids", - label: { - text: "Organisations", - }, + label: "Organisations", heading_size: "l", name: "role[organisation_ids][]", select: { diff --git a/app/views/admin/statistics_announcements/_form.html.erb b/app/views/admin/statistics_announcements/_form.html.erb index 8768ee40aa3..53c9872bfc9 100644 --- a/app/views/admin/statistics_announcements/_form.html.erb +++ b/app/views/admin/statistics_announcements/_form.html.erb @@ -53,9 +53,7 @@ <%= render "components/autocomplete", { id: "statistics_announcement_organisations", - label: { - text: "Organisations (required)", - }, + label: "Organisations (required)", heading_size: "l", name: "statistics_announcement[organisation_ids][]", select: { diff --git a/app/views/admin/users/edit.html.erb b/app/views/admin/users/edit.html.erb index 85da202147c..50e4f889c5a 100644 --- a/app/views/admin/users/edit.html.erb +++ b/app/views/admin/users/edit.html.erb @@ -9,9 +9,7 @@ id: "user_world_location_ids", name: "user[world_location_ids][]", include_blank: true, - label: { - text: "Locations", - }, + label: "Locations", heading_size: "l", select: { multiple: true, diff --git a/app/views/components/_autocomplete.html.erb b/app/views/components/_autocomplete.html.erb index fb5fb6fff37..d60e2790c75 100644 --- a/app/views/components/_autocomplete.html.erb +++ b/app/views/components/_autocomplete.html.erb @@ -23,8 +23,9 @@ <%= tag.div class: root_classes, data: data_attributes do %> <%= render "govuk_publishing_components/components/label", { html_for: id, + text: label, heading_size:, - }.merge(label.symbolize_keys) %> + } %> <% if error_items.any? %> <%= render "govuk_publishing_components/components/error_message", { diff --git a/app/views/components/docs/autocomplete.yml b/app/views/components/docs/autocomplete.yml index 19491ccd433..709d17eaf95 100644 --- a/app/views/components/docs/autocomplete.yml +++ b/app/views/components/docs/autocomplete.yml @@ -12,8 +12,7 @@ examples: data: id: autocomplete name: autocomplete - label: - text: Select your country + label: Select your country select: options: - @@ -31,8 +30,7 @@ examples: id: autocomplete name: autocomplete include_blank: true - label: - text: Select your country + label: Select your country select: options: - @@ -50,8 +48,7 @@ examples: id: autocomplete name: autocomplete include_blank: true - label: - text: Select your country + label: Select your country heading_size: xl select: options: @@ -69,8 +66,7 @@ examples: data: id: autocomplete-selected name: autocomplete-selected - label: - text: Select your country + label: Select your country hint: Only a few countries are available select: options: @@ -88,8 +84,7 @@ examples: with_error: data: name: autocomplete-with-error - label: - text: Autocomplete with error + label: Autocomplete with error select: options: - @@ -108,8 +103,7 @@ examples: data: id: autocomplete-multiselect name: autocomplete-multiselect - label: - text: Select your country + label: Select your country select: multiple: true options: @@ -133,8 +127,7 @@ examples: data_attributes: module: not-a-module loose: moose - label: - text: Select your country + label: Select your country select: options: - @@ -151,8 +144,7 @@ examples: data: id: autocomplete-configuration-options name: autocomplete-configuration-options - label: - text: Status + label: Status select: multiple: true options: diff --git a/lib/engines/content_block_manager/app/views/content_block_manager/content_block/shared/_form.html.erb b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/shared/_form.html.erb index efb69556efa..3dc76f145c4 100644 --- a/lib/engines/content_block_manager/app/views/content_block_manager/content_block/shared/_form.html.erb +++ b/lib/engines/content_block_manager/app/views/content_block_manager/content_block/shared/_form.html.erb @@ -31,9 +31,7 @@ name: "content_block/edition[organisation_id]", error_items: errors_for(@form.content_block_edition.errors, "lead_organisation".to_sym), include_blank: true, - label: { - text: "Lead organisation", - }, + label: "Lead organisation", select: { multiple: false, options: taggable_organisations_container, diff --git a/test/components/autocomplete_test.rb b/test/components/autocomplete_test.rb index c362f750c0a..c863ff0edd2 100644 --- a/test/components/autocomplete_test.rb +++ b/test/components/autocomplete_test.rb @@ -9,9 +9,7 @@ def component_data { id: "id", name: "name", - label: { - text: "text", - }, + label: "text", select: { options: [ ["France", "fr"], From b4840d5a4545c0b9b3707a6062a32071c9f91fa1 Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Thu, 20 Feb 2025 10:17:59 +0000 Subject: [PATCH 10/17] Have 'autocomplete' default `name` to `id` if unspecified This is (useful) behaviour defined in the 'select with search' component. Adding it into 'autocomplete' will make it easier to consolidate the components. --- app/views/components/_autocomplete.html.erb | 1 + app/views/components/docs/autocomplete.yml | 11 +++++++++++ test/components/autocomplete_test.rb | 9 +++++++++ 3 files changed, 21 insertions(+) diff --git a/app/views/components/_autocomplete.html.erb b/app/views/components/_autocomplete.html.erb index d60e2790c75..41c2f50eb70 100644 --- a/app/views/components/_autocomplete.html.erb +++ b/app/views/components/_autocomplete.html.erb @@ -1,5 +1,6 @@ <% id ||= "autocomplete-#{SecureRandom.hex(4)}" + name ||= id error_id = "error-#{SecureRandom.hex(4)}" error_items ||= [] aria = error_id if error_items.any? diff --git a/app/views/components/docs/autocomplete.yml b/app/views/components/docs/autocomplete.yml index 709d17eaf95..49f96d73d3a 100644 --- a/app/views/components/docs/autocomplete.yml +++ b/app/views/components/docs/autocomplete.yml @@ -43,6 +43,17 @@ examples: - United Kingdom - uk + with_missing_name: + description: If no name is provided, name defaults to the (required) value of id. + data: + id: dropdown-with-different-id-and-name + label: My Dropdown + options: + - text: Option one + value: option1 + - text: Option two + value: option2 + with_custom_heading_size: data: id: autocomplete diff --git a/test/components/autocomplete_test.rb b/test/components/autocomplete_test.rb index c863ff0edd2..ea03c2b1a04 100644 --- a/test/components/autocomplete_test.rb +++ b/test/components/autocomplete_test.rb @@ -26,6 +26,15 @@ def component_data end end + test "defaults the 'name' to be the same as 'id'" do + data_without_name = component_data.dup + data_without_name.delete(:name) + render_component(data_without_name) + assert_select ".app-c-autocomplete" + assert_select ".govuk-select[id='id'][name='id']" + assert_select ".govuk-label", text: "text" + end + test "renders the basic component" do render_component(component_data) assert_select ".app-c-autocomplete" From a33aa251dd7b962a3ff725b41f7c8320e2c0cdba Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Thu, 20 Feb 2025 10:41:59 +0000 Subject: [PATCH 11/17] Support custom 'data' attributes in 'select with search' component This is a feature of the 'autocomplete' component that needs carrying over to 'select with search' to consolidate the components. --- app/views/components/docs/select_with_search.yml | 14 ++++++++++++++ .../presenters/select_with_search_helper.rb | 8 +++++--- .../components/select_with_search_test.rb | 8 ++++++++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/app/views/components/docs/select_with_search.yml b/app/views/components/docs/select_with_search.yml index 6b466965419..87636cb1c4d 100644 --- a/app/views/components/docs/select_with_search.yml +++ b/app/views/components/docs/select_with_search.yml @@ -120,6 +120,20 @@ examples: value: option1 - text: Option two value: option2 + with_data_attributes: + data: + id: dropdown-with-data-attributes + data_attributes: + module: not-a-module + loose: moose + label: Select your country + options: + - text: France + value: fr + - text: Germany + value: de + - text: United Kingdom + value: uk with_preselect: data: id: dropdown-with-preselect diff --git a/lib/govuk_publishing_components/presenters/select_with_search_helper.rb b/lib/govuk_publishing_components/presenters/select_with_search_helper.rb index 7a939fdfed4..00f22d1be7d 100644 --- a/lib/govuk_publishing_components/presenters/select_with_search_helper.rb +++ b/lib/govuk_publishing_components/presenters/select_with_search_helper.rb @@ -46,9 +46,11 @@ def options_html end def data_attributes - { - "module": "select-with-search", - }.compact + data_attributes = @local_assigns[:data_attributes] || {} + data_attributes[:module] ||= "" + data_attributes[:module] << " select-with-search" + data_attributes[:module].strip! + data_attributes end def grouped_and_ungrouped_options_for_select(unsorted_options) diff --git a/test/integration/components/select_with_search_test.rb b/test/integration/components/select_with_search_test.rb index c06f2b89223..766dfa8bf36 100644 --- a/test/integration/components/select_with_search_test.rb +++ b/test/integration/components/select_with_search_test.rb @@ -44,4 +44,12 @@ def rendered_options assert_equal rendered_options, %w[Cardiff Swansea] end end + + test "it renders custom data attributes" do + load_example "with_data_attributes" + assert_selector ".app-c-select-with-search" do |node| + assert node[:'data-module'] == "not-a-module select-with-search" + assert node[:'data-loose'] == "moose" + end + end end From 7504bedc5f4f1b651a153f469bb79de9606176dc Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Tue, 18 Feb 2025 16:27:52 +0000 Subject: [PATCH 12/17] Remove unused 'TaggableContentHelper' methods These don't appear to be used anywhere. --- app/helpers/admin/taggable_content_helper.rb | 45 ------------------- .../admin/taggable_content_helper_test.rb | 45 ------------------- 2 files changed, 90 deletions(-) diff --git a/app/helpers/admin/taggable_content_helper.rb b/app/helpers/admin/taggable_content_helper.rb index 35ae9847eb5..630dd59363e 100644 --- a/app/helpers/admin/taggable_content_helper.rb +++ b/app/helpers/admin/taggable_content_helper.rb @@ -1,15 +1,6 @@ # A bunch of helpers for efficiently generating select options for taggable # content, e.g. topics, organisations, etc. module Admin::TaggableContentHelper - # Returns an Array that represents the current set of taggable topical - # events. Each element of the array consists of two values: the name and ID - # of the topical event. - def taggable_topical_events_container - Rails.cache.fetch(taggable_topical_events_cache_digest, expires_in: 1.day) do - TopicalEvent.order(:name).map { |te| [te.name, te.id] } - end - end - # Returns an Array that represents the current set of taggable organisations. # Each element of the array consists of two values: the select_name and the # ID of the organisation @@ -54,17 +45,6 @@ def taggable_role_appointments_container end end - # Returns an Array that represents the taggable ministerial roles. Each - # element of the array consists of two values: the name of the ministerial - # role with the organisation and current holder and its ID. - def taggable_ministerial_roles_container - Rails.cache.fetch(taggable_ministerial_roles_cache_digest, expires_in: 1.day) do - MinisterialRole.with_translations.with_translations_for(:organisations).alphabetical_by_person.map do |role| - ["#{role.name}, #{role.organisations.map(&:name).to_sentence} (#{role.current_person_name})", role.id] - end - end - end - # Returns an Array that represents the current set of taggable detauled # guides. Each element of the array consists of two values: the guide title # and its ID. @@ -112,17 +92,6 @@ def taggable_alternative_format_providers_container end end - # Returns an Array representing the taggable document collections and their - # groups. Each element of the array consists of two values: the - # collection/group name and the ID of the group. - def taggable_document_collection_groups_container - Rails.cache.fetch(taggable_document_collection_groups_cache_digest, expires_in: 1.day) do - DocumentCollection.latest_edition.alphabetical.includes(:groups).flat_map do |collection| - collection.groups.map { |group| ["#{collection.title} (#{group.heading})", group.id] } - end - end - end - # Returns an Array that represents the taggable worldwide organisations. # Each element of the array consists of two values: the name of the worldwide # organisation and its ID. @@ -166,13 +135,6 @@ def taggable_role_appointments_cache_digest @taggable_role_appointments_cache_digest ||= calculate_digest(RoleAppointment.order(:id), "role-appointments") end - # Returns an MD5 digest representing the current set of taggable ministerial - # rile appointments. THis will change if any ministerial role is added or - # updated. - def taggable_ministerial_roles_cache_digest - @taggable_ministerial_roles_cache_digest ||= calculate_digest(MinisterialRole.order(:id), "ministerial-roles") - end - # Returns an MD5 digest representing all the detailed guides. This wil change # if any detailed guides are added or updated. def taggable_detailed_guides_cache_digest @@ -204,13 +166,6 @@ def taggable_alternative_format_providers_cache_digest @taggable_alternative_format_providers_cache_digest ||= calculate_digest(Organisation.order(:id), "alternative-format-providers") end - # Returns an MD5 digest representing the taggable document collection - # groups. This will change if any document collection or group within - # the collection is changed or any new ones are added. - def taggable_document_collection_groups_cache_digest - @taggable_document_collection_groups_cache_digest ||= calculate_digest(Document.where(document_type: "DocumentCollection").order(:id), "document-collection-groups") - end - # Returns an MD5 digest representing the taggable worldwide organisations. This # will change if any worldwide organisation is added or updated. def taggable_worldwide_organisations_cache_digest diff --git a/test/unit/app/helpers/admin/taggable_content_helper_test.rb b/test/unit/app/helpers/admin/taggable_content_helper_test.rb index 0b568e87c7e..3a0cbdc0d83 100644 --- a/test/unit/app/helpers/admin/taggable_content_helper_test.rb +++ b/test/unit/app/helpers/admin/taggable_content_helper_test.rb @@ -1,19 +1,6 @@ require "test_helper" class Admin::TaggableContentHelperTest < ActionView::TestCase - test "#taggable_topical_events_container returns an array of name/ID pairs for all TopicalEvents" do - event_a = create(:topical_event, name: "Event A") - event_c = create(:topical_event, name: "Event C") - event_b = create(:topical_event, name: "Event B") - - assert_equal [ - ["Event A", event_a.id], - ["Event B", event_b.id], - ["Event C", event_c.id], - ], - taggable_topical_events_container - end - test "#taggable_organisations_container returns an array of select_name/ID pairs for all Organisations" do organisation_c = create(:organisation, name: "Organisation C", acronym: "OC") organisation_b = create(:organisation, name: "Organisation B", acronym: "OB") @@ -114,23 +101,6 @@ class Admin::TaggableContentHelperTest < ActionView::TestCase taggable_role_appointments_container end - test "#taggable_ministerial_roles_container returns an array of label/ID pairs for all the ministerial roles" do - create(:board_member_role) - minister_b = create(:ministerial_role, name: "Minister B", organisations: [create(:organisation, name: "Jazz Ministry")]) - minister_a = create(:ministerial_role, name: "Minister A", organisations: minister_b.organisations) - minister_c = create(:ministerial_role, name: "Minister C", organisations: [create(:organisation, name: "Ministry of Outer Space")]) - - create(:role_appointment, role: minister_a, person: create(:person, forename: "Sun", surname: "Ra")) - create(:role_appointment, role: minister_c, person: create(:person, forename: "George", surname: "Clinton")) - - assert_equal [ - ["Minister B, Jazz Ministry (Minister B)", minister_b.id], - ["Minister C, Ministry of Outer Space (George Clinton)", minister_c.id], - ["Minister A, Jazz Ministry (Sun Ra)", minister_a.id], - ], - taggable_ministerial_roles_container - end - test "#taggable_detailed_guides_container returns an array of label/ID pairs for all active detailed guides" do guide_b = create(:published_detailed_guide, title: "Guide B") guide_a = create(:draft_detailed_guide, title: "Guide A") @@ -185,21 +155,6 @@ class Admin::TaggableContentHelperTest < ActionView::TestCase taggable_alternative_format_providers_container end - test "#taggable_document_collection_groups_container returns an array of label/ID pairs for document collection groups" do - group1 = create(:document_collection_group, heading: "Group 1") - group2 = create(:document_collection_group, heading: "Group 2") - group3 = create(:document_collection_group, heading: "Group 3") - create(:document_collection, title: "Collection 1", groups: [group1]) - create(:document_collection, title: "Collection 2", groups: [group2, group3]) - - assert_equal [ - ["Collection 1 (Group 1)", group1.id], - ["Collection 2 (Group 2)", group2.id], - ["Collection 2 (Group 3)", group3.id], - ], - taggable_document_collection_groups_container - end - test "#taggable_ministerial_role_appointments_cache_digest changes when a role appointment is updated" do role_appointment = Timecop.travel 1.year.ago do create(:ministerial_role_appointment, started_at: 1.day.ago) From 0a2f1b732f499d8009279647007acd31f6210cb6 Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Wed, 19 Feb 2025 16:16:18 +0000 Subject: [PATCH 13/17] Change how TaggableContentHelper caches queries The query caching (added in #1712 and #1728) is important to retain as, according to the linked PRs, it causes considerable savings in page load times for the affected pages. However, in one of the next commits we'll be changing the structure of the returned data to include not only the `text` and `value` of each option, but also its `selected` status. We don't want to cache the `selected` status as that would mean for example we're showing the same "Lead Organisation" selected item across all five "Lead organisations" dropdowns on the "New document" page. So, we're keeping the caching, but we're just going to cache the expensive bit - the database result. The `map` calls are very quick in comparison and don't need to be cached. --- app/helpers/admin/taggable_content_helper.rb | 102 ++++++++++++++----- 1 file changed, 75 insertions(+), 27 deletions(-) diff --git a/app/helpers/admin/taggable_content_helper.rb b/app/helpers/admin/taggable_content_helper.rb index 630dd59363e..4bd0d0dbfe9 100644 --- a/app/helpers/admin/taggable_content_helper.rb +++ b/app/helpers/admin/taggable_content_helper.rb @@ -5,8 +5,8 @@ module Admin::TaggableContentHelper # Each element of the array consists of two values: the select_name and the # ID of the organisation def taggable_organisations_container - Rails.cache.fetch(taggable_organisations_cache_digest, expires_in: 1.day) do - Organisation.with_translations.order("organisation_translations.name").map { |o| [o.select_name, o.id] } + cached_taggable_organisations.map do |o| + [o.select_name, o.id] end end @@ -16,16 +16,14 @@ def taggable_organisations_container # role, the date the role was held if it's in the past, and the organisations # the person belongs to) and the ID of the role appointment. def taggable_ministerial_role_appointments_container - Rails.cache.fetch(taggable_ministerial_role_appointments_cache_digest, expires_in: 1.day) do - role_appointments_container_for(RoleAppointment.for_ministerial_roles) + cached_taggable_ministerial_role_appointments.map do |appointment| + [role_appointment_label(appointment), appointment.id] end end def taggable_needs_container - Rails.cache.fetch("need.linkables", expires_in: 1.minute) do - Services.publishing_api.get_linkables(document_type: "need").to_a.map do |need| - need.values_at("title", "content_id") - end + Services.publishing_api.get_linkables(document_type: "need").to_a.map do |need| + need.values_at("title", "content_id") end rescue GdsApi::TimedOutException, GdsApi::HTTPServerError stale_data = Rails.cache.fetch("need.linkables") @@ -40,8 +38,8 @@ def taggable_needs_container # held if it's in the past, and the organisations the person belongs to) and # the ID of the role appointment. def taggable_role_appointments_container - Rails.cache.fetch(taggable_role_appointments_cache_digest, expires_in: 1.day) do - role_appointments_container_for(RoleAppointment) + cached_taggable_role_appointments.map do |appointment| + [role_appointment_label(appointment), appointment.id] end end @@ -49,8 +47,8 @@ def taggable_role_appointments_container # guides. Each element of the array consists of two values: the guide title # and its ID. def taggable_detailed_guides_container - Rails.cache.fetch(taggable_detailed_guides_cache_digest, expires_in: 1.day) do - DetailedGuide.alphabetical.latest_edition.active.map { |d| [d.title, d.id] } + cached_taggable_detailed_guides.map do |d| + [d.title, d.id] end end @@ -58,26 +56,24 @@ def taggable_detailed_guides_container # data sets. Each elements of the array consists of two values: the data # set title and its ID. def taggable_statistical_data_sets_container - Rails.cache.fetch(taggable_statistical_data_sets_cache_digest, expires_in: 1.day) do - StatisticalDataSet.with_translations.latest_edition.map do |data_set| - [data_set.title, data_set.document_id] - end + cached_taggable_statistical_data_sets.map do |data_set| + [data_set.title, data_set.document_id] end end # Returns an Array that represents the taggable world locations. Each element # of the array consists of two values: the location name and its ID def taggable_world_locations_container - Rails.cache.fetch(taggable_world_locations_cache_digest, expires_in: 1.day) do - WorldLocation.ordered_by_name.where(active: true).map { |w| [w.name, w.id] } + cached_taggable_world_locations.map do |w| + [w.name, w.id] end end # Returns an Array that represents the taggable roles. Each element of the # array consists of two values: the role name and its ID def taggable_roles_container - Rails.cache.fetch(taggable_roles_cache_digest, expires_in: 1.day) do - Role.order(:name).map { |w| [w.name, w.id] } + cached_taggable_roles.map do |w| + [w.name, w.id] end end @@ -85,10 +81,8 @@ def taggable_roles_container # Each element of the array consists of two values: the label (organisation # and the email address if avaiable) and the ID of the organisation. def taggable_alternative_format_providers_container - Rails.cache.fetch(taggable_alternative_format_providers_cache_digest, expires_in: 1.day) do - Organisation.alphabetical.map do |o| - ["#{o.name} (#{o.alternative_format_contact_email.presence || '-'})", o.id] - end + cached_taggable_alternative_format_providers.map do |o| + ["#{o.name} (#{o.alternative_format_contact_email.presence || '-'})", o.id] end end @@ -96,8 +90,14 @@ def taggable_alternative_format_providers_container # Each element of the array consists of two values: the name of the worldwide # organisation and its ID. def taggable_worldwide_organisations_container - Rails.cache.fetch(taggable_worldwide_organisations_cache_digest, expires_in: 1.day) do - WorldwideOrganisation.with_translations.latest_edition.map { |wo| [wo.title, wo.document.id] } + cached_taggable_worldwide_organisations.map do |wo| + [wo.title, wo.document.id] + end + end + + def cached_taggable_organisations + Rails.cache.fetch(taggable_organisations_cache_digest, expires_in: 1.day) do + Organisation.with_translations.order("organisation_translations.name") end end @@ -115,6 +115,12 @@ def taggable_organisations_cache_digest @taggable_organisations_cache_digest ||= calculate_digest(Organisation.order(:id), "organisations") end + def cached_taggable_ministerial_role_appointments + Rails.cache.fetch(taggable_ministerial_role_appointments_cache_digest, expires_in: 1.day) do + role_appointments_container_for(RoleAppointment.for_ministerial_roles) + end + end + # Returns an MD5 digest representing the current set of taggable ministerial # role appointments. This will change if any role appointments are added or # changed, and also if an occupied MinisterialRole is updated. @@ -128,6 +134,12 @@ def taggable_ministerial_role_appointments_cache_digest ) end + def cached_taggable_role_appointments + Rails.cache.fetch(taggable_role_appointments_cache_digest, expires_in: 1.day) do + role_appointments_container_for(RoleAppointment) + end + end + # Returns an MD5 digest representing the current set of taggable ministerial # role appointments. This will change if any role appointments are added or # changed, and also if an occupied Role is updated. @@ -135,30 +147,60 @@ def taggable_role_appointments_cache_digest @taggable_role_appointments_cache_digest ||= calculate_digest(RoleAppointment.order(:id), "role-appointments") end + def cached_taggable_detailed_guides + Rails.cache.fetch(taggable_detailed_guides_cache_digest, expires_in: 1.day) do + DetailedGuide.alphabetical.latest_edition.active + end + end + # Returns an MD5 digest representing all the detailed guides. This wil change # if any detailed guides are added or updated. def taggable_detailed_guides_cache_digest @taggable_detailed_guides_cache_digest ||= calculate_digest(Document.where(document_type: "DetailedGuide").order(:id), "detailed-guides") end + def cached_taggable_statistical_data_sets + Rails.cache.fetch(taggable_statistical_data_sets_cache_digest, expires_in: 1.day) do + StatisticalDataSet.with_translations.latest_edition + end + end + # Returns an MD5 digest representing the taggable statistical data sets. This # will change if any statistical data set is added or updated. def taggable_statistical_data_sets_cache_digest @taggable_statistical_data_sets_cache_digest ||= calculate_digest(Document.where(document_type: "StatisticalDataSet").order(:id), "statistical-data-sets") end + def cached_taggable_world_locations + Rails.cache.fetch(taggable_world_locations_cache_digest, expires_in: 1.day) do + WorldLocation.ordered_by_name.where(active: true) + end + end + # Returns an MD5 digest representing the taggable world locations. This will # change if any world locations are added or updated. def taggable_world_locations_cache_digest @taggable_world_locations_cache_digest ||= calculate_digest(WorldLocation.order(:id), "world-locations") end + def cached_taggable_roles + Rails.cache.fetch(taggable_roles_cache_digest, expires_in: 1.day) do + Role.order(:name) + end + end + # Returns an MD5 digest representing the taggable roles. This will # change if any world locations are added or updated. def taggable_roles_cache_digest @taggable_roles_cache_digest ||= calculate_digest(Role.order(:id), "roles") end + def cached_taggable_alternative_format_providers + Rails.cache.fetch(taggable_alternative_format_providers_cache_digest, expires_in: 1.day) do + Organisation.alphabetical + end + end + # Returns an MD5 digest representing the taggable alternative format # providers. This will change if any alternative format providers are # changed. @@ -166,6 +208,12 @@ def taggable_alternative_format_providers_cache_digest @taggable_alternative_format_providers_cache_digest ||= calculate_digest(Organisation.order(:id), "alternative-format-providers") end + def cached_taggable_worldwide_organisations + Rails.cache.fetch(taggable_worldwide_organisations_cache_digest, expires_in: 1.day) do + WorldwideOrganisation.with_translations.latest_edition + end + end + # Returns an MD5 digest representing the taggable worldwide organisations. This # will change if any worldwide organisation is added or updated. def taggable_worldwide_organisations_cache_digest @@ -184,7 +232,7 @@ def role_appointments_container_for(scope) .includes(:person) .with_translations_for(:organisations) .with_translations_for(:role) - .ascending_start_date.map { |appointment| [role_appointment_label(appointment), appointment.id] } + .ascending_start_date end def role_appointment_label(appointment) From b30d1808fcba22d7f11f1d69211901479b445481 Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Wed, 19 Feb 2025 08:52:33 +0000 Subject: [PATCH 14/17] Add 'multiple' support to SelectWithSearchHelper The 'autocomplete' component (which uses 'multiple' select) will use the SelectWithSearchHelper class in the next commit. Before it can do so, the helper needs to be able to mark multiple options as "selected" for the autocomplete tests to continue to pass. --- .../presenters/select_with_search_helper.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/govuk_publishing_components/presenters/select_with_search_helper.rb b/lib/govuk_publishing_components/presenters/select_with_search_helper.rb index 00f22d1be7d..02278cd32de 100644 --- a/lib/govuk_publishing_components/presenters/select_with_search_helper.rb +++ b/lib/govuk_publishing_components/presenters/select_with_search_helper.rb @@ -7,7 +7,7 @@ module Presenters class SelectWithSearchHelper include ActionView::Helpers::FormOptionsHelper - attr_reader :options, :selected_option + attr_reader :options, :selected_options delegate :describedby, :error_id, @@ -23,6 +23,7 @@ def initialize(local_assigns) @options = local_assigns[:options] @grouped_options = local_assigns[:grouped_options] @include_blank = local_assigns[:include_blank] + @selected_options = [] @local_assigns = local_assigns end @@ -40,7 +41,7 @@ def options_html blank_option_if_include_blank + options_for_select( transform_options(@options), - selected_option, + selected_options, ) end end @@ -67,15 +68,15 @@ def grouped_and_ungrouped_options_for_select(unsorted_options) end single_options.flatten! - options_for_select(transform_options(single_options), selected_option) + - grouped_options_for_select(transform_grouped_options(grouped_options), selected_option) + options_for_select(transform_options(single_options), selected_options) + + grouped_options_for_select(transform_grouped_options(grouped_options), selected_options) end private def transform_options(options) options.map do |option| - @selected_option = option[:value] if option[:selected] + @selected_options << option[:value] if option[:selected] [ option[:text], option[:value], From 92c5a525dd40b45731584853a3aa6fd620dbca9a Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Tue, 18 Feb 2025 14:56:22 +0000 Subject: [PATCH 15/17] Convert `options` to hash of `text`, `value` & `selected` To bring the data structures for 'autocomplete' in line with the data structures for 'select with search', we've: - Removed the unnecessary layer of `select -> options` so that it's now just `options` - Done away with the array style `["Text", "value"]` in favour of the more expressive `{ text: "Text", value: "value", selected: true }` - Updated all the affected code - NB: I'm not entirely happy with the "selects organisation if selected in filters" test that's been updated as a result, as we're now really just testing that we've stubbed a value, which is a bit meaningless. --- app/helpers/admin/taggable_content_helper.rb | 110 +++++++------ .../admin/detailed_guides/_form.html.erb | 3 +- .../_accessible_attachment_field.html.erb | 6 +- .../editions/_appointment_fields.html.erb | 3 +- .../editions/_organisation_fields.html.erb | 11 +- .../admin/editions/_role_fields.html.erb | 3 +- .../_statistical_data_set_fields.html.erb | 3 +- .../editions/_topical_event_fields.html.erb | 9 +- .../editions/_world_location_fields.html.erb | 3 +- .../_worldwide_organisation_fields.html.erb | 3 +- .../admin/historical_accounts/_form.html.erb | 14 +- app/views/admin/needs/edit.html.erb | 3 +- .../organisations/_closed_fields.html.erb | 14 +- app/views/admin/organisations/_form.html.erb | 14 +- app/views/admin/roles/_form.html.erb | 8 +- .../speeches/_speaker_select_field.html.erb | 8 +- .../statistics_announcements/_form.html.erb | 8 +- app/views/admin/users/edit.html.erb | 9 +- app/views/components/_autocomplete.html.erb | 19 ++- app/views/components/docs/autocomplete.yml | 149 +++++++----------- .../index/filter_options_component.html.erb | 2 +- .../index/filter_options_component.rb | 18 +-- .../content_block/shared/_form.html.erb | 3 +- .../index/filter_options_component_test.rb | 11 +- test/components/autocomplete_test.rb | 24 +-- .../admin/taggable_content_helper_test.rb | 48 +++--- 26 files changed, 247 insertions(+), 259 deletions(-) diff --git a/app/helpers/admin/taggable_content_helper.rb b/app/helpers/admin/taggable_content_helper.rb index 4bd0d0dbfe9..f68e68afee1 100644 --- a/app/helpers/admin/taggable_content_helper.rb +++ b/app/helpers/admin/taggable_content_helper.rb @@ -1,29 +1,34 @@ # A bunch of helpers for efficiently generating select options for taggable # content, e.g. topics, organisations, etc. module Admin::TaggableContentHelper - # Returns an Array that represents the current set of taggable organisations. - # Each element of the array consists of two values: the select_name and the - # ID of the organisation - def taggable_organisations_container + def taggable_organisations_container(selected_ids = []) cached_taggable_organisations.map do |o| - [o.select_name, o.id] + { + text: o.select_name, + value: o.id, + selected: selected_ids.include?(o.id), + } end end - # Returns an Array that represents the current set of taggable ministerial - # role appointments (both past and present). Each element of the array - # consists of two values: a selectable label (consisting of the person, the - # role, the date the role was held if it's in the past, and the organisations - # the person belongs to) and the ID of the role appointment. - def taggable_ministerial_role_appointments_container + def taggable_ministerial_role_appointments_container(selected_ids = []) cached_taggable_ministerial_role_appointments.map do |appointment| - [role_appointment_label(appointment), appointment.id] + { + text: role_appointment_label(appointment), + value: appointment.id, + selected: selected_ids.include?(appointment.id), + } end end - def taggable_needs_container + def taggable_needs_container(selected_ids) Services.publishing_api.get_linkables(document_type: "need").to_a.map do |need| - need.values_at("title", "content_id") + title, content_id = need.values_at("title", "content_id") + { + text: title, + value: content_id, + selected: selected_ids.include?(content_id), + } end rescue GdsApi::TimedOutException, GdsApi::HTTPServerError stale_data = Rails.cache.fetch("need.linkables") @@ -32,66 +37,73 @@ def taggable_needs_container raise end - # Returns an Array that represents the current set of taggable roles (both - # past and present). Each element of the array consists of two values: a - # selectable label (consisting of the person, the role, the date the role was - # held if it's in the past, and the organisations the person belongs to) and - # the ID of the role appointment. - def taggable_role_appointments_container + def taggable_role_appointments_container(selected_ids = []) cached_taggable_role_appointments.map do |appointment| - [role_appointment_label(appointment), appointment.id] + { + text: role_appointment_label(appointment), + value: appointment.id, + selected: selected_ids.include?(appointment.id), + } end end - # Returns an Array that represents the current set of taggable detauled - # guides. Each element of the array consists of two values: the guide title - # and its ID. - def taggable_detailed_guides_container + def taggable_detailed_guides_container(selected_ids = []) cached_taggable_detailed_guides.map do |d| - [d.title, d.id] + { + text: d.title, + value: d.id, + selected: selected_ids.include?(d.id), + } end end - # Returns an Array that represents the current set of taggable statistical - # data sets. Each elements of the array consists of two values: the data - # set title and its ID. - def taggable_statistical_data_sets_container + def taggable_statistical_data_sets_container(selected_ids = []) cached_taggable_statistical_data_sets.map do |data_set| - [data_set.title, data_set.document_id] + { + text: data_set.title, + value: data_set.document_id, + selected: selected_ids.include?(data_set.document_id), + } end end - # Returns an Array that represents the taggable world locations. Each element - # of the array consists of two values: the location name and its ID - def taggable_world_locations_container + def taggable_world_locations_container(selected_ids = []) cached_taggable_world_locations.map do |w| - [w.name, w.id] + { + text: w.name, + value: w.id, + selected: selected_ids.include?(w.id), + } end end - # Returns an Array that represents the taggable roles. Each element of the - # array consists of two values: the role name and its ID - def taggable_roles_container + def taggable_roles_container(selected_ids = []) cached_taggable_roles.map do |w| - [w.name, w.id] + { + text: w.name, + value: w.id, + selected: selected_ids.include?(w.id), + } end end - # Returns an Array that represents the taggable alternative format providers. - # Each element of the array consists of two values: the label (organisation - # and the email address if avaiable) and the ID of the organisation. - def taggable_alternative_format_providers_container + def taggable_alternative_format_providers_container(selected_ids = []) cached_taggable_alternative_format_providers.map do |o| - ["#{o.name} (#{o.alternative_format_contact_email.presence || '-'})", o.id] + { + text: "#{o.name} (#{o.alternative_format_contact_email.presence || '-'})", + value: o.id, + selected: selected_ids.include?(o.id), + } end end - # Returns an Array that represents the taggable worldwide organisations. - # Each element of the array consists of two values: the name of the worldwide - # organisation and its ID. - def taggable_worldwide_organisations_container + def taggable_worldwide_organisations_container(selected_ids = []) cached_taggable_worldwide_organisations.map do |wo| - [wo.title, wo.document.id] + { + text: wo.title, + value: wo.document.id, + selected: selected_ids.include?(wo.document.id), + } end end diff --git a/app/views/admin/detailed_guides/_form.html.erb b/app/views/admin/detailed_guides/_form.html.erb index d770dd0c544..5e6f952d45f 100644 --- a/app/views/admin/detailed_guides/_form.html.erb +++ b/app/views/admin/detailed_guides/_form.html.erb @@ -22,10 +22,9 @@ include_blank: true, label: "Related guides", heading_size: "m", + options: taggable_detailed_guides_container(edition.related_detailed_guide_ids), select: { - options: taggable_detailed_guides_container, multiple: true, - selected: edition.related_detailed_guide_ids, }, } %> <% end %> diff --git a/app/views/admin/editions/_accessible_attachment_field.html.erb b/app/views/admin/editions/_accessible_attachment_field.html.erb index 1b1ae33c8d2..3d4e9f98091 100644 --- a/app/views/admin/editions/_accessible_attachment_field.html.erb +++ b/app/views/admin/editions/_accessible_attachment_field.html.erb @@ -11,7 +11,11 @@