From 69e149bcd6061fa1c42c29cca7ddee29323562b3 Mon Sep 17 00:00:00 2001 From: James Mead Date: Wed, 20 Sep 2023 16:34:29 +0100 Subject: [PATCH] WIP: Add ga4-index data attrs in Ruby vs JS We're now able to provide the index from the facet partials thus avoiding the need for the code in GOVUK.analyticsGa4.Ga4FinderTracker.setFilterIndexes(). However, I've left the code to trigger the "ga4-filter-indexes-added" event for now to reduce the scope of this change. The plan is that the whole of setFilterIndexes() will become redundant in a subsequent commit. Note that the index is 0-indexed vs 1-indexed since the former is how the Rails indexing was working. This means we currently have to add 1 in the facet partials when setting the ga4-index.index_section value. It might make more sense to change the index in FacetsIterator to be 1-indexed and change OptionSelectFacet#closed_by_default? to work with this. TODO: I'm not completely convinced that the Facet#has_ga4_section? method belongs in the model. --- .../analytics-ga4/ga4-finder-tracker.js | 13 ---------- app/controllers/finders_controller.rb | 2 +- app/lib/facets_iterator.rb | 14 +++++++++++ app/models/facet.rb | 4 +++ app/views/finders/_date_facet.html.erb | 2 +- app/views/finders/_facet_collection.html.erb | 4 +-- .../finders/_option_select_facet.html.erb | 25 +++++++++---------- app/views/finders/_radio_facet.html.erb | 2 +- app/views/finders/_taxon_facet.html.erb | 2 +- .../analytics-ga4/ga4-finder-tracker.spec.js | 20 --------------- 10 files changed, 36 insertions(+), 52 deletions(-) create mode 100644 app/lib/facets_iterator.rb diff --git a/app/assets/javascripts/analytics-ga4/ga4-finder-tracker.js b/app/assets/javascripts/analytics-ga4/ga4-finder-tracker.js index 4693fec0a..40890b4af 100644 --- a/app/assets/javascripts/analytics-ga4/ga4-finder-tracker.js +++ b/app/assets/javascripts/analytics-ga4/ga4-finder-tracker.js @@ -7,20 +7,7 @@ GOVUK.analyticsGa4.Ga4FinderTracker = { - // Finds the parent div containing the filters. Loops through each child div that has data-ga4-filter-parent on it . Sets an index on each of these child divs. setFilterIndexes: function () { - var filterContainer = document.querySelector('[data-ga4-filter-container]') - - if (!filterContainer) { - return - } - - var filterParents = filterContainer.querySelectorAll('[data-ga4-filter-parent]') - - for (var i = 0; i < filterParents.length; i++) { - filterParents[i].setAttribute('data-ga4-index', JSON.stringify({ index_section: i + 1, index_section_count: filterParents.length })) - } - window.GOVUK.triggerEvent(window, 'ga4-filter-indexes-added') }, diff --git a/app/controllers/finders_controller.rb b/app/controllers/finders_controller.rb index f02622148..044c1f6c1 100644 --- a/app/controllers/finders_controller.rb +++ b/app/controllers/finders_controller.rb @@ -113,7 +113,7 @@ def all_facets end def facets - all_facets.select(&:filterable?) + FacetsIterator.new(all_facets.select(&:filterable?)) end def signup_links diff --git a/app/lib/facets_iterator.rb b/app/lib/facets_iterator.rb new file mode 100644 index 000000000..01b035d02 --- /dev/null +++ b/app/lib/facets_iterator.rb @@ -0,0 +1,14 @@ +class FacetsIterator + delegate :select, :any?, to: :@facets + + def initialize(facets) + @facets = facets + @facets_with_ga4_section = @facets.select(&:has_ga4_section?) + end + + def each_with_index_and_count + @facets.each do |facet| + yield facet, @facets_with_ga4_section.index(facet), @facets_with_ga4_section.count + end + end +end diff --git a/app/models/facet.rb b/app/models/facet.rb index 0429ef555..a2a3a65f1 100644 --- a/app/models/facet.rb +++ b/app/models/facet.rb @@ -59,6 +59,10 @@ def ga4_section nil end + def has_ga4_section? + !ga4_section.nil? + end + private def and_word_connectors diff --git a/app/views/finders/_date_facet.html.erb b/app/views/finders/_date_facet.html.erb index 4e6904dd7..f1d33cad8 100644 --- a/app/views/finders/_date_facet.html.erb +++ b/app/views/finders/_date_facet.html.erb @@ -7,6 +7,6 @@ aria_controls_id: "js-search-results-info", date_errors_to: date_facet.error_message_to(@search_query), date_errors_from: date_facet.error_message_from(@search_query), - data_attributes: { "ga4-filter-parent": date_facet.ga4_section } + data_attributes: { "ga4-filter-parent": date_facet.ga4_section, "ga4-index": { index_section: index + 1, index_section_count: count } } } %> \ No newline at end of file diff --git a/app/views/finders/_facet_collection.html.erb b/app/views/finders/_facet_collection.html.erb index f6cd3f521..b86892cc3 100644 --- a/app/views/finders/_facet_collection.html.erb +++ b/app/views/finders/_facet_collection.html.erb @@ -46,8 +46,8 @@
- <% facets.each.with_index do |facet, index| %> - <%= render partial: facet, object: facet, locals: { index: } %> + <% facets.each_with_index_and_count do |facet, index, count| %> + <%= render partial: facet, object: facet, locals: { index:, count: } %> <% end %>
<%= render "facet_tags", facet_tags.present %> diff --git a/app/views/finders/_option_select_facet.html.erb b/app/views/finders/_option_select_facet.html.erb index fe9144062..f78484069 100644 --- a/app/views/finders/_option_select_facet.html.erb +++ b/app/views/finders/_option_select_facet.html.erb @@ -7,17 +7,16 @@ <% add_gem_component_stylesheet("checkboxes") %> <% cache_if option_select_facet.cacheable?, option_select_facet.cache_key do %> - <%= tag.div(data: { "ga4-change-category": "update-filter checkbox", "ga4-filter-parent": "", "ga4-section": option_select_facet.ga4_section }) do %> - <%= render partial: 'govuk_publishing_components/components/option_select', locals: { - :key => option_select_facet.key, - :title => option_select_facet.name, - :aria_controls_id => "js-search-results-info", - :options_container_id => option_select_facet.key, - :options => option_select_facet.options("js-search-results-info", option_select_facet.key), - :closed_on_load => option_select_facet.closed_on_load?(index), - :closed_on_load_mobile => option_select_facet.closed_on_load_mobile?, - :show_filter => option_select_facet.show_option_select_filter, - :large => option_select_facet.large? - } %> - <% end %> + <%= render partial: 'govuk_publishing_components/components/option_select', locals: { + :key => option_select_facet.key, + :title => option_select_facet.name, + :aria_controls_id => "js-search-results-info", + :options_container_id => option_select_facet.key, + :options => option_select_facet.options("js-search-results-info", option_select_facet.key), + :closed_on_load => option_select_facet.closed_on_load?(index), + :closed_on_load_mobile => option_select_facet.closed_on_load_mobile?, + :show_filter => option_select_facet.show_option_select_filter, + :large => option_select_facet.large?, + :data_attributes => { "ga4-change-category": "update-filter checkbox", "ga4-filter-parent": "", "ga4-section": option_select_facet.ga4_section, "ga4-index": { index_section: index + 1, index_section_count: count } } + } %> <% end %> diff --git a/app/views/finders/_radio_facet.html.erb b/app/views/finders/_radio_facet.html.erb index 14e4d79c1..cd21b9424 100644 --- a/app/views/finders/_radio_facet.html.erb +++ b/app/views/finders/_radio_facet.html.erb @@ -1,4 +1,4 @@ -<%= tag.div(class: "facet-container", data: { "ga4-change-category": "update-filter radio", "ga4-section": "", "ga4-filter-parent": radio_facet.ga4_section }) do %> +<%= tag.div(class: "facet-container", data: { "ga4-change-category": "update-filter radio", "ga4-filter-parent": radio_facet.ga4_section, "ga4-section": "", "ga4-index": { index_section: index + 1, index_section_count: count } }) do %> <%= render "govuk_publishing_components/components/radio", { name: radio_facet.key, small: true, diff --git a/app/views/finders/_taxon_facet.html.erb b/app/views/finders/_taxon_facet.html.erb index 96d7f5b4b..175094365 100644 --- a/app/views/finders/_taxon_facet.html.erb +++ b/app/views/finders/_taxon_facet.html.erb @@ -1,7 +1,7 @@ <% unless i_am_a_topic_page_finder %> <%= render "components/expander", { title: "Topic", - data_attributes: { "ga4-filter-parent": taxon_facet.ga4_section } + data_attributes: { "ga4-filter-parent": taxon_facet.ga4_section, "ga4-index": { index_section: index + 1, index_section_count: count } } } do %>
<%= render "govuk_publishing_components/components/select", { diff --git a/spec/javascripts/analytics-ga4/ga4-finder-tracker.spec.js b/spec/javascripts/analytics-ga4/ga4-finder-tracker.spec.js index c927a1f97..dbf39d2c2 100644 --- a/spec/javascripts/analytics-ga4/ga4-finder-tracker.spec.js +++ b/spec/javascripts/analytics-ga4/ga4-finder-tracker.spec.js @@ -349,26 +349,6 @@ describe('GA4 finder change tracker', function () { expect(window.dataLayer[0]).toEqual(expected) }) - it('sets the correct indexes', function () { - form.setAttribute('data-ga4-filter-container', '') - - for (var i = 0; i < 5; i++) { - var div = document.createElement('div') - div.setAttribute('data-ga4-filter-parent', '') - form.appendChild(div) - } - - // Grabs each data-ga4-filter-parent element within a data-ga4-filter-container, and sets the appropriate indexes. - window.GOVUK.analyticsGa4.Ga4FinderTracker.setFilterIndexes() - - var divs = form.querySelectorAll('[data-ga4-filter-parent]') - - for (i = 0; i < divs.length; i++) { - var expectedIndexObject = { index_section: i + 1, index_section_count: divs.length, index_link: undefined } - expect(divs[i].getAttribute('data-ga4-index')).toEqual(JSON.stringify(expectedIndexObject)) - } - }) - it('does nothing if the elementType or changeType does not exist', function () { inputParent = document.createElement('div') input = document.createElement('input')