From 2d0f206e8ad084bb217276cce9958bf100e5ebfd Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Tue, 4 Jun 2024 10:41:38 +1000
Subject: [PATCH 01/25] Prepare spec

---
 .../system/admin/products_v3/products_spec.rb | 37 +++++++++----------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb
index 7cd4a9e8c5..9f9ee35ecd 100644
--- a/spec/system/admin/products_v3/products_spec.rb
+++ b/spec/system/admin/products_v3/products_spec.rb
@@ -30,29 +30,26 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi
     end
   end
 
-  describe "using the page" do
-    describe "using column display dropdown" do
-      let(:product) { create(:simple_product) }
+  describe "column selector" do
+    let!(:product) { create(:simple_product) }
 
-      before do
-        pending "Pending implementation, issue #11055"
-        login_as_admin
-        visit spree.admin_products_path
-      end
+    before do
+      visit admin_products_url
+    end
 
-      it "shows a column display dropdown, which shows a list of columns when clicked" do
-        expect(page).to have_selector "th", text: "NAME"
-        expect(page).to have_selector "th", text: "PRODUCER"
-        expect(page).to have_selector "th", text: "PRICE"
-        expect(page).to have_selector "th", text: "ON HAND"
+    it "shows a column display dropdown, which shows a list of columns when clicked" do
+      expect(page).to have_selector "th", text: "Name"
+      expect(page).to have_selector "th", text: "Producer"
+      expect(page).to have_selector "th", text: "Price"
+      expect(page).to have_selector "th", text: "On Hand"
 
-        toggle_columns /^.{0,1}Producer$/i
+      pending "Pending implementation, issue #11055"
+      toggle_columns /^.{0,1}Producer$/i
 
-        expect(page).not_to have_selector "th", text: "PRODUCER"
-        expect(page).to have_selector "th", text: "NAME"
-        expect(page).to have_selector "th", text: "PRICE"
-        expect(page).to have_selector "th", text: "ON HAND"
-      end
+      expect(page).not_to have_selector "th", text: "Name"
+      expect(page).to have_selector "th",     text: "Producer"
+      expect(page).to have_selector "th",     text: "Price"
+      expect(page).to have_selector "th",     text: "On Hand"
     end
   end
 
@@ -321,6 +318,8 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi
     end
   end
 
+  describe "columns"
+
   describe "updating" do
     let!(:variant_a1) {
       product_a.variants.first.tap{ |v|

From d34e7dbf9fc4cbc414fba324e30c17c48d1ff7ef Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Tue, 4 Jun 2024 12:19:50 +1000
Subject: [PATCH 02/25] Specify accepted format on client side

I don't know why, but even though the client sends http accept header for json, rails is treating it as html. This was being overridden in the route, but I want to support multiple formats next. So, we explicitly choose the format by adding it to the request path.
---
 .../javascripts/admin/index_utils/services/columns.js.coffee    | 2 +-
 config/routes/admin.rb                                          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/admin/index_utils/services/columns.js.coffee b/app/assets/javascripts/admin/index_utils/services/columns.js.coffee
index 2d7abfcab7..aeb1a3e3b6 100644
--- a/app/assets/javascripts/admin/index_utils/services/columns.js.coffee
+++ b/app/assets/javascripts/admin/index_utils/services/columns.js.coffee
@@ -31,7 +31,7 @@ angular.module("admin.indexUtils").factory 'Columns', ($rootScope, $http, $injec
     savePreferences: (action_name) =>
       $http
         method: "PUT"
-        url: "/admin/column_preferences/bulk_update"
+        url: "/admin/column_preferences/bulk_update.json"
         data:
           action_name: action_name
           column_preferences: (preference for column_name, preference of @columns)
diff --git a/config/routes/admin.rb b/config/routes/admin.rb
index 21caa2b677..4016f8a695 100644
--- a/config/routes/admin.rb
+++ b/config/routes/admin.rb
@@ -95,7 +95,7 @@ Openfoodnetwork::Application.routes.draw do
 
     resource :contents
 
-    resources :column_preferences, only: [], format: :json do
+    resources :column_preferences, only: [] do
       put :bulk_update, on: :collection
     end
 

From 9c0f55ad224aa68945ff97bd641a639ec94b4f7a Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Tue, 4 Jun 2024 12:53:13 +1000
Subject: [PATCH 03/25] Refactor

---
 app/controllers/admin/column_preferences_controller.rb | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/app/controllers/admin/column_preferences_controller.rb b/app/controllers/admin/column_preferences_controller.rb
index fcf627f1b7..688dfdb750 100644
--- a/app/controllers/admin/column_preferences_controller.rb
+++ b/app/controllers/admin/column_preferences_controller.rb
@@ -28,11 +28,10 @@ module Admin
     end
 
     def load_collection
-      collection_hash = Hash[permitted_params[:column_preferences].
+      collection_attributes = Hash[permitted_params[:column_preferences].
         each_with_index.map { |cp, i| [i, cp] }]
-      collection_hash.select!{ |_i, cp| cp[:action_name] == permitted_params[:action_name] }
-      @cp_set = Sets::ColumnPreferenceSet.new(@column_preferences,
-                                              collection_attributes: collection_hash)
+      collection_attributes.select!{ |_i, cp| cp[:action_name] == permitted_params[:action_name] }
+      @cp_set = Sets::ColumnPreferenceSet.new(@column_preferences, collection_attributes:)
     end
 
     def collection

From 68da9c9e04c9981e08229f4029b2379ee2f071ab Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Mon, 3 Jun 2024 17:39:44 +1000
Subject: [PATCH 04/25] Add form to save column preferences

---
 .../admin/column_preferences_controller.rb    | 35 ++++++++---
 app/views/admin/products_v3/_sort.html.haml   | 14 +++++
 config/locales/en.yml                         |  3 +-
 .../column_preference_defaults.rb             | 19 ++++++
 .../column_preferences_controller_spec.rb     | 58 ++++++++++++++-----
 .../system/admin/products_v3/products_spec.rb |  5 ++
 6 files changed, 112 insertions(+), 22 deletions(-)

diff --git a/app/controllers/admin/column_preferences_controller.rb b/app/controllers/admin/column_preferences_controller.rb
index 688dfdb750..3f405cfd90 100644
--- a/app/controllers/admin/column_preferences_controller.rb
+++ b/app/controllers/admin/column_preferences_controller.rb
@@ -4,17 +4,23 @@ module Admin
   class ColumnPreferencesController < Admin::ResourceController
     before_action :load_collection, only: [:bulk_update]
 
-    respond_to :json
-
     def bulk_update
       @cp_set.collection.each { |cp| authorize! :bulk_update, cp }
 
       if @cp_set.save
-        render json: @cp_set.collection, each_serializer: Api::Admin::ColumnPreferenceSerializer
+        respond_to do |format|
+          format.json { render json: @cp_set.collection, each_serializer: Api::Admin::ColumnPreferenceSerializer }
+          format.html { render inline: "saved" } #todo
+        end
       elsif @cp_set.errors.present?
-        render json: { errors: @cp_set.errors }, status: :bad_request
+        respond_to do |format|
+          format.json { render json: { errors: @cp_set.errors }, status: :bad_request }
+          format.html { render inline: "errors" } #todo
+        end
       else
-        render body: nil, status: :internal_server_error
+        respond_to do |format|
+          format.all { render body: nil, status: :internal_server_error }
+        end
       end
     end
 
@@ -28,9 +34,22 @@ module Admin
     end
 
     def load_collection
-      collection_attributes = Hash[permitted_params[:column_preferences].
-        each_with_index.map { |cp, i| [i, cp] }]
-      collection_attributes.select!{ |_i, cp| cp[:action_name] == permitted_params[:action_name] }
+      collection_attributes = nil
+
+      respond_to do |format|
+        format.json do
+          collection_attributes = Hash[permitted_params[:column_preferences].
+            each_with_index.map { |cp, i| [i, cp] }]
+          collection_attributes.select!{ |_i, cp| cp[:action_name] == permitted_params[:action_name] }
+        end
+        format.html do
+          # Inject action name and user ID for each column_preference
+          collection_attributes = permitted_params[:column_preferences].to_h.each_value { |cp|
+            cp[:action_name] = permitted_params[:action_name]
+            cp[:user_id] = spree_current_user.id
+          }
+        end
+      end
       @cp_set = Sets::ColumnPreferenceSet.new(@column_preferences, collection_attributes:)
     end
 
diff --git a/app/views/admin/products_v3/_sort.html.haml b/app/views/admin/products_v3/_sort.html.haml
index f53eec4e76..a8a3f1b335 100644
--- a/app/views/admin/products_v3/_sort.html.haml
+++ b/app/views/admin/products_v3/_sort.html.haml
@@ -13,3 +13,17 @@
         options_for_select([15, 25, 50, 100].collect{|i| [t('.pagination.per_page.per_page', num: i), i]}, pagy&.items),
         class: "no-input per-page",
         data: { controller: "tom-select search", action: "change->search#changePerPage", "tom-select-options-value": '{ "plugins": [] }'}
+
+  / Columns dropdown
+  = form_with url: bulk_update_admin_column_preferences_path, method: :put do |f|
+    = hidden_field_tag :action_name, "#{controller_name}_#{action_name}"
+
+    - ColumnPreference.for(spree_current_user, "#{controller_name}_#{action_name}").each do |column_preference|
+      = f.fields_for("column_preferences[]", column_preference) do |cp_form|
+        = cp_form.hidden_field :id
+        = cp_form.hidden_field :column_name
+        %label
+          = cp_form.check_box :visible
+          = t("admin.products_page.columns." + column_preference.column_name)
+
+    = f.submit t('admin.column_save_as_default')
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 47b4b07814..41c76b2cc8 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -571,6 +571,7 @@ en:
         per_page: "%{count} items per page"
         colums: Columns
       columns:
+        image: Image
         name: Name
         unit_scale: Unit scale
         unit: Unit
@@ -661,7 +662,7 @@ en:
     choose: "Choose..."
     please_select: Please select...
 
-    column_save_as_default: Save As Default
+    column_save_as_default: Save as default
     columns: Columns
     actions: Actions
     viewing: "Viewing: %{current_view_name}"
diff --git a/lib/open_food_network/column_preference_defaults.rb b/lib/open_food_network/column_preference_defaults.rb
index 9bb09b2061..39fc0edf69 100644
--- a/lib/open_food_network/column_preference_defaults.rb
+++ b/lib/open_food_network/column_preference_defaults.rb
@@ -77,6 +77,25 @@ module OpenFoodNetwork
       }
     end
 
+    def products_v3_index_columns
+      # TODO: Use consistent translation keys
+      node = "admin.products_page.columns"
+      {
+        image: { name: I18n.t("admin.image"), visible: true },
+        producer: { name: I18n.t("admin.producer"), visible: true },
+        sku: { name: I18n.t("admin.sku"), visible: false },
+        name: { name: I18n.t("admin.name"),                    visible: true },
+        unit: { name: I18n.t("#{node}.unit"),                  visible: true },
+        price: { name: I18n.t("admin.price"), visible: true },
+        on_hand: { name: I18n.t("admin.on_hand"), visible: true },
+        on_demand: { name: I18n.t("admin.on_demand"), visible: true },
+        category: { name: I18n.t("#{node}.category"), visible: false },
+        tax_category: { name: I18n.t("#{node}.tax_category"), visible: false },
+        inherits_properties: { name: I18n.t("#{node}.inherits_properties"), visible: false },
+        import_date: { name: I18n.t("#{node}.import_date"), visible: false }
+      }
+    end
+
     def enterprises_index_columns
       node = "admin.enterprises.index"
       {
diff --git a/spec/controllers/admin/column_preferences_controller_spec.rb b/spec/controllers/admin/column_preferences_controller_spec.rb
index 41f34b163e..5cd8707bb0 100644
--- a/spec/controllers/admin/column_preferences_controller_spec.rb
+++ b/spec/controllers/admin/column_preferences_controller_spec.rb
@@ -9,13 +9,26 @@ RSpec.describe Admin::ColumnPreferencesController, type: :controller do
     let!(:user1) { create(:user) }
     let!(:user2) { create(:user) }
     let!(:enterprise) { create(:enterprise, owner: user1, users: [user1, user2]) }
+    let!(:column_preference) {
+      ColumnPreference.create(user_id: user1.id, action_name: 'enterprises_index',
+                              column_name: "name", visible: true)
+    }
+
+    shared_examples "where I own the preferences submitted" do
+      before do
+        allow(controller).to receive(:spree_current_user) { user1 }
+      end
+
+      it "allows me to update the column preferences" do
+        spree_put :bulk_update, format: request_format, action_name: "enterprises_index",
+                                column_preferences: column_preference_params
+        expect(ColumnPreference.where(user_id: user1.id,
+                                      action_name: 'enterprises_index').count).to be 3
+      end
+    end
 
     context "json" do
-      let!(:column_preference) {
-        ColumnPreference.create(user_id: user1.id, action_name: 'enterprises_index',
-                                column_name: "name", visible: true)
-      }
-
+      let(:request_format) { :json }
       let(:column_preference_params) {
         [
           { id: column_preference.id, user_id: user1.id, action_name: "enterprises_index",
@@ -27,28 +40,47 @@ RSpec.describe Admin::ColumnPreferencesController, type: :controller do
         ]
       }
 
+      it_behaves_like "where I own the preferences submitted"
+
       context "where I don't own the preferences submitted" do
         before do
           allow(controller).to receive(:spree_current_user) { user2 }
         end
 
         it "prevents me from updating the column preferences" do
-          spree_put :bulk_update, format: :json, action_name: "enterprises_index",
+          spree_put :bulk_update, format: request_format, action_name: "enterprises_index",
                                   column_preferences: column_preference_params
           expect(ColumnPreference.count).to be 1
         end
       end
+    end
 
-      context "where I own the preferences submitted" do
+    context "html" do
+      let(:request_format) { :html }
+      let(:column_preference_params) {
+        {
+          '0': { id: column_preference.id, column_name: "name", visible: "0" },
+          '1': { id: nil, column_name: "producer", visible: "1" },
+          '2': { id: nil, column_name: "status", visible: "1" },
+        }
+      }
+
+      it_behaves_like "where I own the preferences submitted"
+
+      context "where I don't own the preferences submitted" do
         before do
-          allow(controller).to receive(:spree_current_user) { user1 }
+          allow(controller).to receive(:spree_current_user) { user2 }
         end
 
-        it "allows me to update the column preferences" do
-          spree_put :bulk_update, format: :json, action_name: "enterprises_index",
-                                  column_preferences: column_preference_params
-          expect(ColumnPreference.where(user_id: user1.id,
-                                        action_name: 'enterprises_index').count).to be 3
+        # This has the same effect as the JSON action, but due to differing implementation,
+        # it has different expections.
+        it "prevents me from updating the column preferences" do
+          expect {
+            spree_put :bulk_update, format: request_format, action_name: "enterprises_index",
+                                    column_preferences: column_preference_params
+          }.to raise_error(ActiveRecord::RecordNotUnique)
+
+          expect(column_preference.reload.visible).to eq true
         end
       end
     end
diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb
index 9f9ee35ecd..73db0e4bca 100644
--- a/spec/system/admin/products_v3/products_spec.rb
+++ b/spec/system/admin/products_v3/products_spec.rb
@@ -43,6 +43,11 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi
       expect(page).to have_selector "th", text: "Price"
       expect(page).to have_selector "th", text: "On Hand"
 
+      uncheck "Name"
+      click_on "Save as default"
+      refresh
+      expect(page).to have_unchecked_field "Name"
+
       pending "Pending implementation, issue #11055"
       toggle_columns /^.{0,1}Producer$/i
 

From 50469fe53e990f8290d4c53132bce6e044848692 Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Wed, 5 Jun 2024 09:57:50 +1000
Subject: [PATCH 05/25] Use consistent translation keys
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Ensures that the column table names match the names in the selector.

I thought 'there must be a way to set the translation scope once'. With Rails, there's usually a way. Thankfully this one was quite simple. Or is it too much magic.. 🧙

https://coderwall.com/p/dvme9q/set-scope-of-i18n-translations-in-rails-with-a-block
---
 .../column_preference_defaults.rb             | 30 +++++++++----------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/lib/open_food_network/column_preference_defaults.rb b/lib/open_food_network/column_preference_defaults.rb
index 39fc0edf69..f02d5c63d1 100644
--- a/lib/open_food_network/column_preference_defaults.rb
+++ b/lib/open_food_network/column_preference_defaults.rb
@@ -78,22 +78,20 @@ module OpenFoodNetwork
     end
 
     def products_v3_index_columns
-      # TODO: Use consistent translation keys
-      node = "admin.products_page.columns"
-      {
-        image: { name: I18n.t("admin.image"), visible: true },
-        producer: { name: I18n.t("admin.producer"), visible: true },
-        sku: { name: I18n.t("admin.sku"), visible: false },
-        name: { name: I18n.t("admin.name"),                    visible: true },
-        unit: { name: I18n.t("#{node}.unit"),                  visible: true },
-        price: { name: I18n.t("admin.price"), visible: true },
-        on_hand: { name: I18n.t("admin.on_hand"), visible: true },
-        on_demand: { name: I18n.t("admin.on_demand"), visible: true },
-        category: { name: I18n.t("#{node}.category"), visible: false },
-        tax_category: { name: I18n.t("#{node}.tax_category"), visible: false },
-        inherits_properties: { name: I18n.t("#{node}.inherits_properties"), visible: false },
-        import_date: { name: I18n.t("#{node}.import_date"), visible: false }
-      }
+      I18n.with_options scope: 'admin.products_page.columns' do
+        {
+          image: { name: t(:image), visible: true },
+          name: { name: t(:name), visible: true },
+          sku: { name: t(:sku), visible: false },
+          unit: { name: t(:unit), visible: true },
+          price: { name: t(:price), visible: true },
+          on_hand: { name: t(:on_hand), visible: true },
+          producer: { name: t(:producer), visible: true },
+          category: { name: t(:category), visible: false },
+          tax_category: { name: t(:tax_category), visible: false },
+          inherits_properties: { name: t(:inherits_properties), visible: false },
+        }
+      end
     end
 
     def enterprises_index_columns

From 89cedc42870566981bd8b34e90e11606b7c15406 Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Tue, 4 Jun 2024 15:59:10 +1000
Subject: [PATCH 06/25] Submit and render with Turbo Stream

---
 .../admin/column_preferences_controller.rb          | 11 ++++++++---
 app/views/admin/column_preferences/_form.html.haml  | 12 ++++++++++++
 .../bulk_update.turbo_stream.haml                   |  3 +++
 app/views/admin/products_v3/_sort.html.haml         | 13 +------------
 config/locales/en.yml                               |  4 ++++
 .../admin/column_preferences_controller_spec.rb     |  4 ++--
 6 files changed, 30 insertions(+), 17 deletions(-)
 create mode 100644 app/views/admin/column_preferences/_form.html.haml
 create mode 100644 app/views/admin/column_preferences/bulk_update.turbo_stream.haml

diff --git a/app/controllers/admin/column_preferences_controller.rb b/app/controllers/admin/column_preferences_controller.rb
index 3f405cfd90..9d39e050fb 100644
--- a/app/controllers/admin/column_preferences_controller.rb
+++ b/app/controllers/admin/column_preferences_controller.rb
@@ -10,12 +10,17 @@ module Admin
       if @cp_set.save
         respond_to do |format|
           format.json { render json: @cp_set.collection, each_serializer: Api::Admin::ColumnPreferenceSerializer }
-          format.html { render inline: "saved" } #todo
+          format.turbo_stream {
+            flash.now[:success] = t('.success')
+            render :bulk_update, locals: { action: permitted_params[:action_name] } }
         end
       elsif @cp_set.errors.present?
         respond_to do |format|
           format.json { render json: { errors: @cp_set.errors }, status: :bad_request }
-          format.html { render inline: "errors" } #todo
+          format.turbo_stream {
+            flash.now[:error] = t('.error')
+            render :bulk_update, locals: { action: permitted_params[:action_name] }
+          }
         end
       else
         respond_to do |format|
@@ -42,7 +47,7 @@ module Admin
             each_with_index.map { |cp, i| [i, cp] }]
           collection_attributes.select!{ |_i, cp| cp[:action_name] == permitted_params[:action_name] }
         end
-        format.html do
+        format.all do
           # Inject action name and user ID for each column_preference
           collection_attributes = permitted_params[:column_preferences].to_h.each_value { |cp|
             cp[:action_name] = permitted_params[:action_name]
diff --git a/app/views/admin/column_preferences/_form.html.haml b/app/views/admin/column_preferences/_form.html.haml
new file mode 100644
index 0000000000..7e58ba68eb
--- /dev/null
+++ b/app/views/admin/column_preferences/_form.html.haml
@@ -0,0 +1,12 @@
+= form_with url: bulk_update_admin_column_preferences_path, method: :put, id: :bulk_admin_column_preferences_form do |f|
+  = hidden_field_tag :action_name, action
+
+  - ColumnPreference.for(spree_current_user, action).each_with_index do |column_preference, index|
+    = f.fields_for("column_preferences", column_preference, index:) do |cp_form|
+      = cp_form.hidden_field :id
+      = cp_form.hidden_field :column_name
+      %label
+        = cp_form.check_box :visible
+        = t("admin.products_page.columns." + column_preference.column_name)
+
+  = f.submit t('admin.column_save_as_default')
diff --git a/app/views/admin/column_preferences/bulk_update.turbo_stream.haml b/app/views/admin/column_preferences/bulk_update.turbo_stream.haml
new file mode 100644
index 0000000000..70836e4657
--- /dev/null
+++ b/app/views/admin/column_preferences/bulk_update.turbo_stream.haml
@@ -0,0 +1,3 @@
+= turbo_stream.replace "bulk_admin_column_preferences_form" do
+  = render partial: "admin/shared/flashes", locals: { flashes: flash } if defined? flash
+  = render partial: 'form', locals: { action: }
diff --git a/app/views/admin/products_v3/_sort.html.haml b/app/views/admin/products_v3/_sort.html.haml
index a8a3f1b335..f5d3d6329d 100644
--- a/app/views/admin/products_v3/_sort.html.haml
+++ b/app/views/admin/products_v3/_sort.html.haml
@@ -15,15 +15,4 @@
         data: { controller: "tom-select search", action: "change->search#changePerPage", "tom-select-options-value": '{ "plugins": [] }'}
 
   / Columns dropdown
-  = form_with url: bulk_update_admin_column_preferences_path, method: :put do |f|
-    = hidden_field_tag :action_name, "#{controller_name}_#{action_name}"
-
-    - ColumnPreference.for(spree_current_user, "#{controller_name}_#{action_name}").each do |column_preference|
-      = f.fields_for("column_preferences[]", column_preference) do |cp_form|
-        = cp_form.hidden_field :id
-        = cp_form.hidden_field :column_name
-        %label
-          = cp_form.check_box :visible
-          = t("admin.products_page.columns." + column_preference.column_name)
-
-    = f.submit t('admin.column_save_as_default')
+  = render partial: "admin/column_preferences/form", locals: { action: "products_v3_index" }
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 41c76b2cc8..c3bdca88a8 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -761,6 +761,10 @@ en:
         balance_due: "Balance Due"
       destroy:
         has_associated_subscriptions: "Delete failed: This customer has active subscriptions. Cancel them first."
+    column_preferences:
+      bulk_update:
+        success: "Column preferences saved"
+        error: "Column preferences could not be saved"
     contents:
       edit:
         title: Content
diff --git a/spec/controllers/admin/column_preferences_controller_spec.rb b/spec/controllers/admin/column_preferences_controller_spec.rb
index 5cd8707bb0..c29153534d 100644
--- a/spec/controllers/admin/column_preferences_controller_spec.rb
+++ b/spec/controllers/admin/column_preferences_controller_spec.rb
@@ -55,8 +55,8 @@ RSpec.describe Admin::ColumnPreferencesController, type: :controller do
       end
     end
 
-    context "html" do
-      let(:request_format) { :html }
+    context "turbo_stream" do
+      let(:request_format) { :turbo_stream }
       let(:column_preference_params) {
         {
           '0': { id: column_preference.id, column_name: "name", visible: "0" },

From d81c3cb489d09222e3be6ccd0bc7711f2c2158e3 Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Wed, 5 Jun 2024 12:50:26 +1000
Subject: [PATCH 07/25] Show/hide columns based on checkboxes

The cols could have been a lot cleaner with simple classnames, but I preferred to mark up in a way that reveals the purpose (otherwise they could be used for styling).

It doesn't seem to be any faster comparing querySelector('[data]') vs class, or iterating through the dom nodes.
---
 .../admin/column_preferences/_form.html.haml  |  6 ++--
 app/views/admin/products_v3/_table.html.haml  | 22 ++++++-------
 .../column_preferences_controller.js          | 33 +++++++++++++++++++
 .../column_preference_defaults.rb             |  1 +
 .../system/admin/products_v3/products_spec.rb | 11 ++++---
 5 files changed, 56 insertions(+), 17 deletions(-)
 create mode 100644 app/webpacker/controllers/column_preferences_controller.js

diff --git a/app/views/admin/column_preferences/_form.html.haml b/app/views/admin/column_preferences/_form.html.haml
index 7e58ba68eb..969f4a3a9c 100644
--- a/app/views/admin/column_preferences/_form.html.haml
+++ b/app/views/admin/column_preferences/_form.html.haml
@@ -1,4 +1,6 @@
-= form_with url: bulk_update_admin_column_preferences_path, method: :put, id: :bulk_admin_column_preferences_form do |f|
+= form_with url: bulk_update_admin_column_preferences_path, method: :put,
+    id: :bulk_admin_column_preferences_form,
+    html: { 'data-controller': "column-preferences" } do |f|
   = hidden_field_tag :action_name, action
 
   - ColumnPreference.for(spree_current_user, action).each_with_index do |column_preference, index|
@@ -6,7 +8,7 @@
       = cp_form.hidden_field :id
       = cp_form.hidden_field :column_name
       %label
-        = cp_form.check_box :visible
+        = cp_form.check_box :visible, 'data-column-name': column_preference.column_name
         = t("admin.products_page.columns." + column_preference.column_name)
 
   = f.submit t('admin.column_save_as_default')
diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml
index 464130d181..eb917c0ad8 100644
--- a/app/views/admin/products_v3/_table.html.haml
+++ b/app/views/admin/products_v3/_table.html.haml
@@ -15,17 +15,17 @@
 
   %table.products
     %colgroup
-      %col{ width:"56" }= #  Img (size + padding)
-      %col= # (grow to fill) Name
-      %col{ width:"5%"}
-      %col{ width:"8%"}
-      %col{ width:"8%"}
-      %col{ width:"5%"}
-      %col{ width:"10%"}
-      %col{ width:"15%"}= # Producer
-      %col{ width:"8%"}
-      %col{ width:"8%"}
-      %col{ width:"8%"}
+      %col{ width:"56", 'data-column-preferences-name': :image }= # (size + padding)
+      %col{ 'data-column-preferences-name': :name }= # (grow to fill)
+      %col{ width:"5%", 'data-column-preferences-name': :sku }
+      %col{ width:"8%", 'data-column-preferences-name': :unit_scale }
+      %col{ width:"8%", 'data-column-preferences-name': :unit }
+      %col{ width:"5%", 'data-column-preferences-name': :price}
+      %col{ width:"10%", 'data-column-preferences-name': :on_hand}
+      %col{ width:"15%", 'data-column-preferences-name': :producer}
+      %col{ width:"8%", 'data-column-preferences-name': :category}
+      %col{ width:"8%", 'data-column-preferences-name': :tax_category}
+      %col{ width:"8%", 'data-column-preferences-name': :inherits_properties}
       %col{ width:"8%"}= # Actions
     %thead
       %tr
diff --git a/app/webpacker/controllers/column_preferences_controller.js b/app/webpacker/controllers/column_preferences_controller.js
new file mode 100644
index 0000000000..31ec0bb65c
--- /dev/null
+++ b/app/webpacker/controllers/column_preferences_controller.js
@@ -0,0 +1,33 @@
+import { Controller } from "stimulus";
+
+
+// Manage column visibility according to checkbox selection
+//
+export default class ColumnPreferencesController extends Controller {
+  connect() {
+    this.checkboxes = this.element.querySelectorAll('input[type=checkbox]');
+
+    for (const element of this.checkboxes) {
+      // On initial load
+      this.#showHideColumn(element);
+      // On checkbox changed
+      element.addEventListener("change", this.#showHideColumn.bind(this));
+    }
+  }
+
+  // private
+
+  #showHideColumn(e) {
+    const element = e.target || e;
+    const name = element.dataset.columnName;
+    const selector = `col[data-column-preferences-name="${name}"]`;
+    const column = document.querySelector(selector);
+
+    if (column == null) {
+      console.error(`ColumnPreferencesController: could not find ${selector}`);
+      return;
+    }
+
+    column.style.visibility = (element.checked ? '' : 'collapse');
+  }
+}
diff --git a/lib/open_food_network/column_preference_defaults.rb b/lib/open_food_network/column_preference_defaults.rb
index f02d5c63d1..7451145d10 100644
--- a/lib/open_food_network/column_preference_defaults.rb
+++ b/lib/open_food_network/column_preference_defaults.rb
@@ -84,6 +84,7 @@ module OpenFoodNetwork
           name: { name: t(:name), visible: true },
           sku: { name: t(:sku), visible: false },
           unit: { name: t(:unit), visible: true },
+          unit_scale: { name: t(:unit_scale), visible: true },
           price: { name: t(:price), visible: true },
           on_hand: { name: t(:on_hand), visible: true },
           producer: { name: t(:producer), visible: true },
diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb
index 73db0e4bca..076393006b 100644
--- a/spec/system/admin/products_v3/products_spec.rb
+++ b/spec/system/admin/products_v3/products_spec.rb
@@ -37,20 +37,23 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi
       visit admin_products_url
     end
 
-    it "shows a column display dropdown, which shows a list of columns when clicked" do
+    it "hides column and remembers saved preference" do
       expect(page).to have_selector "th", text: "Name"
       expect(page).to have_selector "th", text: "Producer"
       expect(page).to have_selector "th", text: "Price"
       expect(page).to have_selector "th", text: "On Hand"
 
       uncheck "Name"
+
+      expect(page).not_to have_selector "th", text: "Name"
+      expect(page).to have_selector "th",     text: "Producer"
+      expect(page).to have_selector "th",     text: "Price"
+      expect(page).to have_selector "th",     text: "On Hand"
+
       click_on "Save as default"
       refresh
       expect(page).to have_unchecked_field "Name"
 
-      pending "Pending implementation, issue #11055"
-      toggle_columns /^.{0,1}Producer$/i
-
       expect(page).not_to have_selector "th", text: "Name"
       expect(page).to have_selector "th",     text: "Producer"
       expect(page).to have_selector "th",     text: "Price"

From ae66a85cc57d56ac68b3d14a3557744c39f04b1e Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Wed, 5 Jun 2024 14:06:03 +1000
Subject: [PATCH 08/25] Show error messages

There shouldn't normally be errors, but I got one due to bad data during development, and this helped sort it out.
---
 app/controllers/admin/column_preferences_controller.rb | 3 ++-
 config/locales/en.yml                                  | 1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/controllers/admin/column_preferences_controller.rb b/app/controllers/admin/column_preferences_controller.rb
index 9d39e050fb..abe14e4d22 100644
--- a/app/controllers/admin/column_preferences_controller.rb
+++ b/app/controllers/admin/column_preferences_controller.rb
@@ -18,7 +18,7 @@ module Admin
         respond_to do |format|
           format.json { render json: { errors: @cp_set.errors }, status: :bad_request }
           format.turbo_stream {
-            flash.now[:error] = t('.error')
+            flash.now[:error] = @cp_set.errors.full_messages.to_sentence
             render :bulk_update, locals: { action: permitted_params[:action_name] }
           }
         end
@@ -55,6 +55,7 @@ module Admin
           }
         end
       end
+
       @cp_set = Sets::ColumnPreferenceSet.new(@column_preferences, collection_attributes:)
     end
 
diff --git a/config/locales/en.yml b/config/locales/en.yml
index c3bdca88a8..a5881d9c68 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -764,7 +764,6 @@ en:
     column_preferences:
       bulk_update:
         success: "Column preferences saved"
-        error: "Column preferences could not be saved"
     contents:
       edit:
         title: Content

From e516e7f33559a4b9bf7bd49f7156479d30d0b06b Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Wed, 5 Jun 2024 15:19:58 +1000
Subject: [PATCH 09/25] Tweak checkbox dropdown styles

Use the new design for checkboxes and fix alignment.

Removes redesigned-input, which is a small regression on the old design, but I think it's acceptable bcause we're going to shut it down soon.
---
 .../admin/columns_dropdown.html.haml          |  5 ++--
 ...ultiple_checked_select_component.html.haml |  4 +--
 app/webpacker/css/admin/components/input.scss | 29 -------------------
 .../css/admin_v3/components/dropdown.scss     | 12 ++++----
 4 files changed, 12 insertions(+), 38 deletions(-)

diff --git a/app/assets/javascripts/templates/admin/columns_dropdown.html.haml b/app/assets/javascripts/templates/admin/columns_dropdown.html.haml
index 61f42974db..7895d7070e 100644
--- a/app/assets/javascripts/templates/admin/columns_dropdown.html.haml
+++ b/app/assets/javascripts/templates/admin/columns_dropdown.html.haml
@@ -5,8 +5,9 @@
   %div.menu{ 'ng-show' => "expanded" }
     .menu_items
       .menu_item{ "ng-repeat": "column in columns", "ng-click": "toggle(column);" }
-        %input.redesigned-input{ type: "checkbox", "ng-checked": "column.visible" }
-        {{ column.name }}
+        %input{ type: "checkbox", "ng-checked": "column.visible" }
+        %span
+          {{ column.name }}
       %hr
       %div.menu_item.text-center
         %input.fullwidth.orange{ type: "button", "ng-value": "saved() ? 'Saved': 'Saving'", "ng-show": "saved() || saving", "ng-disabled": "saved()" }
diff --git a/app/components/multiple_checked_select_component/multiple_checked_select_component.html.haml b/app/components/multiple_checked_select_component/multiple_checked_select_component.html.haml
index 8dedce27ca..c9c0abebec 100644
--- a/app/components/multiple_checked_select_component/multiple_checked_select_component.html.haml
+++ b/app/components/multiple_checked_select_component/multiple_checked_select_component.html.haml
@@ -9,5 +9,5 @@
     %div.menu_items
       - @options.each do |option|
         %label.menu_item{ "data-multiple-checked-select-target": "option", "data-value": option[1], "data-label": option[0] }
-          %input.redesigned-input{ type: "checkbox", checked: @selected.include?(option[1]), name: "#{@name}[]", value: option[1] }
-          = option[0]
+          %input{ type: "checkbox", checked: @selected.include?(option[1]), name: "#{@name}[]", value: option[1] }
+          %span= option[0]
diff --git a/app/webpacker/css/admin/components/input.scss b/app/webpacker/css/admin/components/input.scss
index 1c1a847e56..2b390bce6d 100644
--- a/app/webpacker/css/admin/components/input.scss
+++ b/app/webpacker/css/admin/components/input.scss
@@ -6,32 +6,3 @@
     }
   }
 }
-
-input[type="checkbox"].redesigned-input {
-  position: relative;
-  top: 1px;
-  -moz-appearance: none;
-  -webkit-appearance: none;
-  -o-appearance: none;
-  appearance: none;
-  outline: none;
-  content: none;
-  cursor: pointer;
-
-  &:before {
-    font-family: "FontAwesome";
-    content: "\f00c";
-    font-size: 15px;
-    color: transparent !important;
-    background: transparent !important;
-    display: block;
-    width: 15px;
-    height: 15px;
-    border: 1px solid #809cb1;
-    margin-right: 7px;
-  }
-
-  &:checked:before {
-    color: $color-txt-text !important;
-  }
-}
diff --git a/app/webpacker/css/admin_v3/components/dropdown.scss b/app/webpacker/css/admin_v3/components/dropdown.scss
index 8a101d1c8d..25531a1467 100644
--- a/app/webpacker/css/admin_v3/components/dropdown.scss
+++ b/app/webpacker/css/admin_v3/components/dropdown.scss
@@ -101,8 +101,6 @@
 
   > span {
     width: auto;
-    text-transform: uppercase;
-    font-size: 85%;
     font-weight: 600;
   }
 
@@ -190,7 +188,6 @@
     display: inline-block;
     list-style: none;
     width: auto;
-    text-transform: uppercase;
     font-size: 85%;
     font-weight: 600;
     margin: -8px -15px;
@@ -212,6 +209,7 @@
   border: 1px solid $lighter-grey;
   background-color: $lighter-grey;
   padding: 0px;
+  line-height: normal;
 
   &:hover {
     border-color: $lighter-grey;
@@ -263,8 +261,12 @@
       cursor: pointer;
       padding-top: 4px;
       padding-bottom: 5px;
-      text-transform: uppercase;
-      font-size: 85%;
+      font-size: inherit;
+
+      // Align checkbox and text
+      & > * {
+        vertical-align: middle;
+      }
     }
   }
 }

From c6452efa92390f54d5aa00be80e3ebf83ec8951c Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Tue, 11 Jun 2024 13:52:05 +1000
Subject: [PATCH 10/25] Isolate styles for their intended use

This also improves the styling of the orders action dropdowns (on index and edit pages). It adds the new chevron icon, but needed some fiddling to make it look right.
---
 .../css/admin_v3/components/dropdown.scss     | 33 ++++++++++++-------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/app/webpacker/css/admin_v3/components/dropdown.scss b/app/webpacker/css/admin_v3/components/dropdown.scss
index 25531a1467..a0b3d745a1 100644
--- a/app/webpacker/css/admin_v3/components/dropdown.scss
+++ b/app/webpacker/css/admin_v3/components/dropdown.scss
@@ -179,6 +179,23 @@
     }
   }
 
+  summary:after {
+    content: "\f077";
+    font-family: FontAwesome;
+    font-size: 13px;
+  }
+
+
+  &[open] >,
+  details[open] > {
+    summary:after {
+      content: "\f078";
+      font-family: FontAwesome;
+    }
+  }
+}
+
+.ofn-drop-down:not(.ofn-dropdown-v2) {
   > details {
     margin: -7px -15px;
     padding: 7px 15px;
@@ -188,20 +205,14 @@
     display: inline-block;
     list-style: none;
     width: auto;
-    font-size: 85%;
-    font-weight: 600;
     margin: -8px -15px;
     padding: 8px 15px;
-  }
 
-  > details > summary:after {
-    content: "\f0d7";
-    font-family: FontAwesome;
-  }
-
-  > details[open] > summary:after {
-    content: "\f0d8";
-    font-family: FontAwesome;
+    &:after {
+      position: relative;
+      bottom: 1px;
+      font-size: 12px;
+    }
   }
 }
 

From 9ba3b4f2d55209858ca9ed6f7f1a67aeaec2e1e8 Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Tue, 11 Jun 2024 15:51:33 +1000
Subject: [PATCH 11/25] Fix up styles

---
 app/views/admin/column_preferences/_form.html.haml |  2 +-
 app/views/admin/products_v3/_sort.html.haml        |  2 +-
 app/webpacker/css/admin/products_v3.scss           | 10 +++++++++-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/app/views/admin/column_preferences/_form.html.haml b/app/views/admin/column_preferences/_form.html.haml
index 969f4a3a9c..88ccd1a981 100644
--- a/app/views/admin/column_preferences/_form.html.haml
+++ b/app/views/admin/column_preferences/_form.html.haml
@@ -1,5 +1,5 @@
 = form_with url: bulk_update_admin_column_preferences_path, method: :put,
-    id: :bulk_admin_column_preferences_form,
+    id: :bulk_admin_column_preferences_form, class: "column-preferences",
     html: { 'data-controller': "column-preferences" } do |f|
   = hidden_field_tag :action_name, action
 
diff --git a/app/views/admin/products_v3/_sort.html.haml b/app/views/admin/products_v3/_sort.html.haml
index f5d3d6329d..6749b3b07a 100644
--- a/app/views/admin/products_v3/_sort.html.haml
+++ b/app/views/admin/products_v3/_sort.html.haml
@@ -1,5 +1,5 @@
 #sort
-  %div
+  %div.pagination-description
     - if pagy.present?
       = t(".pagination.total_html", total: pagy.count, from: pagy.from, to: pagy.to)
 
diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss
index 2b277f0e33..646c2d074b 100644
--- a/app/webpacker/css/admin/products_v3.scss
+++ b/app/webpacker/css/admin/products_v3.scss
@@ -214,6 +214,10 @@
       display: none;
     }
 
+    .pagination-description {
+      flex-grow: 1; // Grow to fill space
+    }
+
     .with-dropdown {
       display: flex;
       justify-content: space-between;
@@ -221,7 +225,11 @@
       gap: 10px;
     }
     .per-page {
-      width: 10em;
+      width: 9em;
+    }
+
+    .column-preferences .ofn-drop-down-label {
+      width: 13em;
     }
   }
 

From 8c75e6baa8de6a69b0efafa6536aa8bf8b259348 Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Tue, 11 Jun 2024 15:27:04 +1000
Subject: [PATCH 12/25] Make column selector a dropdown

With some styling tweaks.
---
 .../admin/column_preferences/_form.html.haml  | 25 +++++++----
 .../controllers/dropdown_controller.js        |  1 +
 .../css/admin_v3/components/dropdown.scss     | 12 ++++-
 spec/support/request/admin_helper.rb          | 12 +++--
 .../system/admin/products_v3/products_spec.rb | 44 ++++++++++++-------
 5 files changed, 65 insertions(+), 29 deletions(-)

diff --git a/app/views/admin/column_preferences/_form.html.haml b/app/views/admin/column_preferences/_form.html.haml
index 88ccd1a981..6296444b02 100644
--- a/app/views/admin/column_preferences/_form.html.haml
+++ b/app/views/admin/column_preferences/_form.html.haml
@@ -3,12 +3,21 @@
     html: { 'data-controller': "column-preferences" } do |f|
   = hidden_field_tag :action_name, action
 
-  - ColumnPreference.for(spree_current_user, action).each_with_index do |column_preference, index|
-    = f.fields_for("column_preferences", column_preference, index:) do |cp_form|
-      = cp_form.hidden_field :id
-      = cp_form.hidden_field :column_name
-      %label
-        = cp_form.check_box :visible, 'data-column-name': column_preference.column_name
-        = t("admin.products_page.columns." + column_preference.column_name)
+  / DC: this makes my Chrome DevTools crash when inspecting the <details> element. If problem continues, we need to use a different method.
+  %details.ofn-drop-down.ofn-drop-down-v2.right{ 'data-controller': "dropdown" }
+    %summary.ofn-drop-down-label
+      = t('admin.columns')
+      %span.icon-caret
 
-  = f.submit t('admin.column_save_as_default')
+    .menu
+      .menu_items
+        - ColumnPreference.for(spree_current_user, action).each_with_index do |column_preference, index|
+          = f.fields_for("column_preferences", column_preference, index:) do |cp_form|
+            = cp_form.hidden_field :id
+            = cp_form.hidden_field :column_name
+            %label.menu_item
+              = cp_form.check_box :visible, 'data-column-name': column_preference.column_name
+              %span= t("admin.products_page.columns." + column_preference.column_name)
+
+      .actions
+        = f.submit t('admin.column_save_as_default'), class: "secondary fullwidth"
diff --git a/app/webpacker/controllers/dropdown_controller.js b/app/webpacker/controllers/dropdown_controller.js
index 844289e12c..701fdf27a7 100644
--- a/app/webpacker/controllers/dropdown_controller.js
+++ b/app/webpacker/controllers/dropdown_controller.js
@@ -1,5 +1,6 @@
 import { Controller } from "stimulus";
 
+// Close a <details> element when click outside
 export default class extends Controller {
 
   connect() {
diff --git a/app/webpacker/css/admin_v3/components/dropdown.scss b/app/webpacker/css/admin_v3/components/dropdown.scss
index a0b3d745a1..32ed4ec4f1 100644
--- a/app/webpacker/css/admin_v3/components/dropdown.scss
+++ b/app/webpacker/css/admin_v3/components/dropdown.scss
@@ -111,7 +111,7 @@
     top: 100%;
     left: 0px;
     padding: 5px 0px;
-    border: 1px solid #adadad;
+    border-radius: $border-radius;
     background-color: #ffffff;
     box-shadow: 1px 3px 10px #888888;
     z-index: 100;
@@ -280,6 +280,16 @@
       }
     }
   }
+
+  .actions {
+    margin-top: 5px;
+    margin-right: 15px; // Compensate for scrollbar on menu_items
+    padding: 2px 10px;
+
+    &:hover {
+      background-color: inherit;
+    }
+  }
 }
 
 .ofn-drop-down.ofn-drop-down-v2 {
diff --git a/spec/support/request/admin_helper.rb b/spec/support/request/admin_helper.rb
index 60e0fcdbd4..eb06f34b12 100644
--- a/spec/support/request/admin_helper.rb
+++ b/spec/support/request/admin_helper.rb
@@ -3,10 +3,10 @@
 module AdminHelper
   def toggle_columns(*labels)
     # open dropdown
-    # case insensitive search for "Columns" text
-    find("div#columns-dropdown", text: /columns/i).click
+    columns_dropdown = ofn_drop_down("Columns")
+    columns_dropdown.click
 
-    within "div#columns-dropdown" do
+    within columns_dropdown do
       labels.each do |label|
         # Convert label to case-insensitive regexp if not one already
         label = /#{label}/i unless label.is_a?(Regexp)
@@ -16,6 +16,10 @@ module AdminHelper
     end
 
     # close dropdown
-    find("div#columns-dropdown", text: /columns/i).click
+    columns_dropdown.click
+  end
+
+  def ofn_drop_down(label)
+    find(".ofn-drop-down", text: /#{label}/i)
   end
 end
diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb
index 076393006b..5d76e3e522 100644
--- a/spec/system/admin/products_v3/products_spec.rb
+++ b/spec/system/admin/products_v3/products_spec.rb
@@ -3,6 +3,7 @@
 require "system_helper"
 
 RSpec.describe 'As an enterprise user, I can manage my products', feature: :admin_style_v3 do
+  include AdminHelper
   include WebHelper
   include AuthenticationHelper
   include FileHelper
@@ -38,26 +39,37 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi
     end
 
     it "hides column and remembers saved preference" do
+      # Name shows by default
+      expect(page).to have_checked_field "Name"
       expect(page).to have_selector "th", text: "Name"
+      expect_other_columns_visible
+
+      # Name is hidden
+      ofn_drop_down("Columns").click
+      within ofn_drop_down("Columns") do
+        uncheck "Name"
+      end
+      # expect(page).not_to have_selector "th", text: "Name" # column is not visible, but capybara doesn't understand yet.
+      expect_other_columns_visible
+
+      # Preference saved
+      click_on "Save as default"
+      expect(page).to have_content "Column preferences saved"
+      refresh
+
+      # Preference remembered
+      ofn_drop_down("Columns").click
+      within ofn_drop_down("Columns") do
+        expect(page).to have_unchecked_field "Name"
+      end
+      # expect(page).not_to have_selector "th", text: "Name"
+      expect_other_columns_visible
+    end
+
+    def expect_other_columns_visible
       expect(page).to have_selector "th", text: "Producer"
       expect(page).to have_selector "th", text: "Price"
       expect(page).to have_selector "th", text: "On Hand"
-
-      uncheck "Name"
-
-      expect(page).not_to have_selector "th", text: "Name"
-      expect(page).to have_selector "th",     text: "Producer"
-      expect(page).to have_selector "th",     text: "Price"
-      expect(page).to have_selector "th",     text: "On Hand"
-
-      click_on "Save as default"
-      refresh
-      expect(page).to have_unchecked_field "Name"
-
-      expect(page).not_to have_selector "th", text: "Name"
-      expect(page).to have_selector "th",     text: "Producer"
-      expect(page).to have_selector "th",     text: "Price"
-      expect(page).to have_selector "th",     text: "On Hand"
     end
   end
 

From 0190d6f31df62761c1d6ea5b0739c6bfe33b851a Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Tue, 11 Jun 2024 15:52:36 +1000
Subject: [PATCH 13/25] Update dropdown styles

The v2 dropdown is used in various places, and now looks more in line with the new design.
---
 .../css/admin_v3/components/dropdown.scss     | 21 +++++++------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/app/webpacker/css/admin_v3/components/dropdown.scss b/app/webpacker/css/admin_v3/components/dropdown.scss
index 32ed4ec4f1..6f8bd99c8a 100644
--- a/app/webpacker/css/admin_v3/components/dropdown.scss
+++ b/app/webpacker/css/admin_v3/components/dropdown.scss
@@ -180,8 +180,10 @@
   }
 
   summary:after {
-    content: "\f077";
+    content: "\f078";
     font-family: FontAwesome;
+    position: relative;
+    top: 3px;
     font-size: 13px;
   }
 
@@ -189,7 +191,7 @@
   &[open] >,
   details[open] > {
     summary:after {
-      content: "\f078";
+      content: "\f077";
       font-family: FontAwesome;
     }
   }
@@ -210,7 +212,7 @@
 
     &:after {
       position: relative;
-      bottom: 1px;
+      top: -1px;
       font-size: 12px;
     }
   }
@@ -221,8 +223,10 @@
   background-color: $lighter-grey;
   padding: 0px;
   line-height: normal;
+  @include border-radius($border-radius);
 
-  &:hover {
+  &:hover,
+  &.expanded {
     border-color: $lighter-grey;
   }
 
@@ -291,12 +295,3 @@
     }
   }
 }
-
-.ofn-drop-down.ofn-drop-down-v2 {
-  // Add very specific styling here for components that are in transition:
-  // ie. the ones using the two classes above
-  .ofn-drop-down-label {
-    padding-top: 7px;
-    padding-bottom: 7px;
-  }
-}

From 70fab2bcc133200817a9fccac41eaf3e836f9bf7 Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Wed, 12 Jun 2024 15:55:13 +1000
Subject: [PATCH 14/25] Show/hide columns using display instead of visibility

Visibility was way simpler, but the table doesn't recalculate column widths until you use display:none;

This is now using the same method as the old products screen.
But we still need to update colspans..
---
 .../column_preferences_controller.js          | 25 ++++++++++++++++---
 .../system/admin/products_v3/products_spec.rb |  4 +--
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/app/webpacker/controllers/column_preferences_controller.js b/app/webpacker/controllers/column_preferences_controller.js
index 31ec0bb65c..9a81faefb6 100644
--- a/app/webpacker/controllers/column_preferences_controller.js
+++ b/app/webpacker/controllers/column_preferences_controller.js
@@ -1,11 +1,10 @@
 import { Controller } from "stimulus";
 
-
 // Manage column visibility according to checkbox selection
 //
 export default class ColumnPreferencesController extends Controller {
   connect() {
-    this.checkboxes = this.element.querySelectorAll('input[type=checkbox]');
+    this.checkboxes = this.element.querySelectorAll("input[type=checkbox]");
 
     for (const element of this.checkboxes) {
       // On initial load
@@ -22,12 +21,32 @@ export default class ColumnPreferencesController extends Controller {
     const name = element.dataset.columnName;
     const selector = `col[data-column-preferences-name="${name}"]`;
     const column = document.querySelector(selector);
+    const index = this.#getIndex(column);
 
     if (column == null) {
       console.error(`ColumnPreferencesController: could not find ${selector}`);
       return;
     }
 
-    column.style.visibility = (element.checked ? '' : 'collapse');
+    // Hide column definition
+    this.#showHideElement(column, element.checked);
+
+    // Hide each cell in column (ignore rows with colspan)
+    const rows = column.closest("table").querySelectorAll("tr:not(:has(td[colspan]))");
+    rows.forEach((row) => {
+      // Ignore cell if spanning multiple columns
+      const cell = row.children[index];
+      if (cell == undefined) return;
+
+      this.#showHideElement(cell, element.checked);
+    });
+  }
+
+  #getIndex(column) {
+    return Array.from(column.parentNode.children).indexOf(column);
+  }
+
+  #showHideElement(element, show) {
+    element.style.display = show ? "" : "none";
   }
 }
diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb
index 5d76e3e522..620e1901a2 100644
--- a/spec/system/admin/products_v3/products_spec.rb
+++ b/spec/system/admin/products_v3/products_spec.rb
@@ -49,7 +49,7 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi
       within ofn_drop_down("Columns") do
         uncheck "Name"
       end
-      # expect(page).not_to have_selector "th", text: "Name" # column is not visible, but capybara doesn't understand yet.
+      expect(page).not_to have_selector "th", text: "Name"
       expect_other_columns_visible
 
       # Preference saved
@@ -62,7 +62,7 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi
       within ofn_drop_down("Columns") do
         expect(page).to have_unchecked_field "Name"
       end
-      # expect(page).not_to have_selector "th", text: "Name"
+      expect(page).not_to have_selector "th", text: "Name"
       expect_other_columns_visible
     end
 

From d5456a85b76761506fb1ee466a5673a8539fc31c Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Wed, 12 Jun 2024 16:58:01 +1000
Subject: [PATCH 15/25] Reset cell colspans

This might be a little simpler if we move the 'new variant' button to col 0, and assume colspan cells always span the whole table.
---
 app/views/admin/products_v3/_table.html.haml  |  2 +-
 .../column_preferences_controller.js          | 20 ++++++++++++++++---
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml
index eb917c0ad8..726158bb2c 100644
--- a/app/views/admin/products_v3/_table.html.haml
+++ b/app/views/admin/products_v3/_table.html.haml
@@ -13,7 +13,7 @@
   = hidden_field_tag :producer_id, @producer_id
   = hidden_field_tag :category_id, @category_id
 
-  %table.products
+  %table.products{ 'data-column-preferences-target': "table" }
     %colgroup
       %col{ width:"56", 'data-column-preferences-name': :image }= # (size + padding)
       %col{ 'data-column-preferences-name': :name }= # (grow to fill)
diff --git a/app/webpacker/controllers/column_preferences_controller.js b/app/webpacker/controllers/column_preferences_controller.js
index 9a81faefb6..dfbcee6f2b 100644
--- a/app/webpacker/controllers/column_preferences_controller.js
+++ b/app/webpacker/controllers/column_preferences_controller.js
@@ -4,8 +4,15 @@ import { Controller } from "stimulus";
 //
 export default class ColumnPreferencesController extends Controller {
   connect() {
-    this.checkboxes = this.element.querySelectorAll("input[type=checkbox]");
+    this.table = document.querySelector('table[data-column-preferences-target="table"]');
+    this.cols = Array.from(this.table.querySelectorAll('col'));
+    this.colSpanCells = this.table.querySelectorAll('th[colspan],td[colspan]');
+    // Initialise data-default-col-span
+    this.colSpanCells.forEach((cell)=> {
+      cell.dataset.defaultColSpan ||= cell.colSpan;
+    });
 
+    this.checkboxes = this.element.querySelectorAll("input[type=checkbox]");
     for (const element of this.checkboxes) {
       // On initial load
       this.#showHideColumn(element);
@@ -20,7 +27,7 @@ export default class ColumnPreferencesController extends Controller {
     const element = e.target || e;
     const name = element.dataset.columnName;
     const selector = `col[data-column-preferences-name="${name}"]`;
-    const column = document.querySelector(selector);
+    const column = this.table.querySelector(selector);
     const index = this.#getIndex(column);
 
     if (column == null) {
@@ -32,7 +39,7 @@ export default class ColumnPreferencesController extends Controller {
     this.#showHideElement(column, element.checked);
 
     // Hide each cell in column (ignore rows with colspan)
-    const rows = column.closest("table").querySelectorAll("tr:not(:has(td[colspan]))");
+    const rows = this.table.querySelectorAll("tr:not(:has(td[colspan]))");
     rows.forEach((row) => {
       // Ignore cell if spanning multiple columns
       const cell = row.children[index];
@@ -40,6 +47,13 @@ export default class ColumnPreferencesController extends Controller {
 
       this.#showHideElement(cell, element.checked);
     });
+
+    // Reset cell colspans
+    const hiddenColCount = this.cols.filter((col)=> col.style.display == 'none').length;
+    for(const cell of this.colSpanCells) {
+      const span = parseInt(cell.dataset.defaultColSpan, 10) - hiddenColCount;
+      cell.colSpan = span;
+    };
   }
 
   #getIndex(column) {

From 9ae4d347aa6983af2de964d44663afcd2d4a5fe0 Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Wed, 12 Jun 2024 17:04:37 +1000
Subject: [PATCH 16/25] Update widths

For some reason, minimum widths work now (I swear they didn't before).

Hmm i would really like to shorten that stimulus controller name.
---
 app/views/admin/products_v3/_table.html.haml | 24 ++++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml
index 726158bb2c..12722e9d30 100644
--- a/app/views/admin/products_v3/_table.html.haml
+++ b/app/views/admin/products_v3/_table.html.haml
@@ -15,18 +15,18 @@
 
   %table.products{ 'data-column-preferences-target': "table" }
     %colgroup
-      %col{ width:"56", 'data-column-preferences-name': :image }= # (size + padding)
-      %col{ 'data-column-preferences-name': :name }= # (grow to fill)
-      %col{ width:"5%", 'data-column-preferences-name': :sku }
-      %col{ width:"8%", 'data-column-preferences-name': :unit_scale }
-      %col{ width:"8%", 'data-column-preferences-name': :unit }
-      %col{ width:"5%", 'data-column-preferences-name': :price}
-      %col{ width:"10%", 'data-column-preferences-name': :on_hand}
-      %col{ width:"15%", 'data-column-preferences-name': :producer}
-      %col{ width:"8%", 'data-column-preferences-name': :category}
-      %col{ width:"8%", 'data-column-preferences-name': :tax_category}
-      %col{ width:"8%", 'data-column-preferences-name': :inherits_properties}
-      %col{ width:"8%"}= # Actions
+      %col{ 'data-column-preferences-name': :image,      width:"1%", style:"min-width: 56px" }= # (size + padding)
+      %col{ 'data-column-preferences-name': :name,       width:"15%", style:"min-width: 6em" }= #  (grow to fill)
+      %col{ 'data-column-preferences-name': :sku,        width:"10%", style:"min-width: 6em" }
+      %col{ 'data-column-preferences-name': :unit_scale, width:"8%" }
+      %col{ 'data-column-preferences-name': :unit,       width:"8%" }
+      %col{ 'data-column-preferences-name': :price,      width:"5%", style:"min-width: 5em" }
+      %col{ 'data-column-preferences-name': :on_hand,    width:"10%"}
+      %col{ 'data-column-preferences-name': :producer,   width:"15%"}
+      %col{ 'data-column-preferences-name': :category,   width:"8%" }
+      %col{ 'data-column-preferences-name': :tax_category, width:"8%" }
+      %col{ 'data-column-preferences-name': :inherits_properties, width:"8%" }
+      %col{ width:"1%", style:"min-width: 5em"}= # Actions
     %thead
       %tr
         %td.form-actions-wrapper{ colspan: 12 }

From e7774d7a247fea30e2257be7a2528b6dddf57c26 Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Thu, 13 Jun 2024 10:39:19 +1000
Subject: [PATCH 17/25] Lint fix

Sorry didn't have time to go back and rebase
---
 .../admin/column_preferences_controller.rb            | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/app/controllers/admin/column_preferences_controller.rb b/app/controllers/admin/column_preferences_controller.rb
index abe14e4d22..80dda11b3a 100644
--- a/app/controllers/admin/column_preferences_controller.rb
+++ b/app/controllers/admin/column_preferences_controller.rb
@@ -9,10 +9,13 @@ module Admin
 
       if @cp_set.save
         respond_to do |format|
-          format.json { render json: @cp_set.collection, each_serializer: Api::Admin::ColumnPreferenceSerializer }
+          format.json {
+            render json: @cp_set.collection, each_serializer: Api::Admin::ColumnPreferenceSerializer
+          }
           format.turbo_stream {
             flash.now[:success] = t('.success')
-            render :bulk_update, locals: { action: permitted_params[:action_name] } }
+            render :bulk_update, locals: { action: permitted_params[:action_name] }
+          }
         end
       elsif @cp_set.errors.present?
         respond_to do |format|
@@ -45,7 +48,9 @@ module Admin
         format.json do
           collection_attributes = Hash[permitted_params[:column_preferences].
             each_with_index.map { |cp, i| [i, cp] }]
-          collection_attributes.select!{ |_i, cp| cp[:action_name] == permitted_params[:action_name] }
+          collection_attributes.select!{ |_i, cp|
+            cp[:action_name] == permitted_params[:action_name]
+          }
         end
         format.all do
           # Inject action name and user ID for each column_preference

From a7ef2432623e759dc6600a3b418fdf9b8dd58cbb Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Thu, 13 Jun 2024 10:43:33 +1000
Subject: [PATCH 18/25] Enable all columns by default

---
 lib/open_food_network/column_preference_defaults.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/open_food_network/column_preference_defaults.rb b/lib/open_food_network/column_preference_defaults.rb
index 7451145d10..f851efc794 100644
--- a/lib/open_food_network/column_preference_defaults.rb
+++ b/lib/open_food_network/column_preference_defaults.rb
@@ -82,15 +82,15 @@ module OpenFoodNetwork
         {
           image: { name: t(:image), visible: true },
           name: { name: t(:name), visible: true },
-          sku: { name: t(:sku), visible: false },
+          sku: { name: t(:sku), visible: true },
           unit: { name: t(:unit), visible: true },
           unit_scale: { name: t(:unit_scale), visible: true },
           price: { name: t(:price), visible: true },
           on_hand: { name: t(:on_hand), visible: true },
           producer: { name: t(:producer), visible: true },
-          category: { name: t(:category), visible: false },
-          tax_category: { name: t(:tax_category), visible: false },
-          inherits_properties: { name: t(:inherits_properties), visible: false },
+          category: { name: t(:category), visible: true },
+          tax_category: { name: t(:tax_category), visible: true },
+          inherits_properties: { name: t(:inherits_properties), visible: true },
         }
       end
     end

From db27fc5a2bc2b0778b89159c2b29bd06b1ba4711 Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Thu, 13 Jun 2024 10:51:00 +1000
Subject: [PATCH 19/25] Remove dead code

I'm pretty sure that case doesn't happen, and besides there's no spec for it.
---
 app/controllers/admin/column_preferences_controller.rb | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/app/controllers/admin/column_preferences_controller.rb b/app/controllers/admin/column_preferences_controller.rb
index 80dda11b3a..e6b0103f57 100644
--- a/app/controllers/admin/column_preferences_controller.rb
+++ b/app/controllers/admin/column_preferences_controller.rb
@@ -17,7 +17,7 @@ module Admin
             render :bulk_update, locals: { action: permitted_params[:action_name] }
           }
         end
-      elsif @cp_set.errors.present?
+      else
         respond_to do |format|
           format.json { render json: { errors: @cp_set.errors }, status: :bad_request }
           format.turbo_stream {
@@ -25,10 +25,6 @@ module Admin
             render :bulk_update, locals: { action: permitted_params[:action_name] }
           }
         end
-      else
-        respond_to do |format|
-          format.all { render body: nil, status: :internal_server_error }
-        end
       end
     end
 

From b25d2ed32aaf4ca742150de0e53b1f0f1b9c0dcc Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Thu, 13 Jun 2024 10:51:35 +1000
Subject: [PATCH 20/25] Refactor to fix Metrics/AbcSize linter

---
 app/controllers/admin/column_preferences_controller.rb | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/app/controllers/admin/column_preferences_controller.rb b/app/controllers/admin/column_preferences_controller.rb
index e6b0103f57..5403a9a5ee 100644
--- a/app/controllers/admin/column_preferences_controller.rb
+++ b/app/controllers/admin/column_preferences_controller.rb
@@ -7,8 +7,8 @@ module Admin
     def bulk_update
       @cp_set.collection.each { |cp| authorize! :bulk_update, cp }
 
-      if @cp_set.save
-        respond_to do |format|
+      respond_to do |format|
+        if @cp_set.save
           format.json {
             render json: @cp_set.collection, each_serializer: Api::Admin::ColumnPreferenceSerializer
           }
@@ -16,9 +16,7 @@ module Admin
             flash.now[:success] = t('.success')
             render :bulk_update, locals: { action: permitted_params[:action_name] }
           }
-        end
-      else
-        respond_to do |format|
+        else
           format.json { render json: { errors: @cp_set.errors }, status: :bad_request }
           format.turbo_stream {
             flash.now[:error] = @cp_set.errors.full_messages.to_sentence

From 5d0e241f8c0fef389ce34341412c3ea886d14de4 Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Thu, 13 Jun 2024 11:04:21 +1000
Subject: [PATCH 21/25] Pending spec

Probably due to column tweaks, revealing different existing problems.
---
 spec/system/admin/products_v3/products_spec.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb
index 620e1901a2..511b7885e2 100644
--- a/spec/system/admin/products_v3/products_spec.rb
+++ b/spec/system/admin/products_v3/products_spec.rb
@@ -1038,6 +1038,7 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi
           tomselect_search_and_select(category_to_select, from: "Category")
 
           sleep(0.1)
+          pending "unable to select tax_category dropdown, possibly because category is still open and too big, covering the tax_cat." # rubocop:disable Layout/LineLength
           validate_tomselect_with_search!(
             page, "Tax Category",
             tax_categories_search_selector

From 3c9f77dc2b5cc92f8daa895efa654b2f0c7b6fc7 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Fri, 14 Jun 2024 09:48:55 +1000
Subject: [PATCH 22/25] Restore image display with absolute width

The `min-width` property is ignored by Firefox. And we don't need the
column to grow any bigger than the picture size anyway. An absolute
width is correct here.

The specification says:

> Applies to all elements but non-replaced inline elements, table rows,
> and row groups.

Firefox is totally right in ignoring it.
---
 app/views/admin/products_v3/_table.html.haml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml
index 12722e9d30..0338784a74 100644
--- a/app/views/admin/products_v3/_table.html.haml
+++ b/app/views/admin/products_v3/_table.html.haml
@@ -15,7 +15,8 @@
 
   %table.products{ 'data-column-preferences-target': "table" }
     %colgroup
-      %col{ 'data-column-preferences-name': :image,      width:"1%", style:"min-width: 56px" }= # (size + padding)
+      -# The `min-width` property works in Chrome but not Firefox.
+      %col{ 'data-column-preferences-name': :image,      width:"56px" }= # (image size + padding)
       %col{ 'data-column-preferences-name': :name,       width:"15%", style:"min-width: 6em" }= #  (grow to fill)
       %col{ 'data-column-preferences-name': :sku,        width:"10%", style:"min-width: 6em" }
       %col{ 'data-column-preferences-name': :unit_scale, width:"8%" }

From aff50f66c401ecdf238ec590d960cf0a14d66a8a Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Mon, 17 Jun 2024 12:14:36 +1000
Subject: [PATCH 23/25] Name and Producer columns grow to fill and other tweaks

If neither are visible, the first column on the left (eg image) will grow. But that's not a likely scenario.

Min-widths help manage sizes on smaller screens in Chrome.
The title for Inherits Properties gets cut off, but I think it's better than cutting off content.

Oh look, it fixed a spec too!
---
 app/views/admin/products_v3/_table.html.haml   | 12 ++++++------
 spec/system/admin/products_v3/products_spec.rb |  1 -
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml
index 0338784a74..d15e471012 100644
--- a/app/views/admin/products_v3/_table.html.haml
+++ b/app/views/admin/products_v3/_table.html.haml
@@ -15,19 +15,19 @@
 
   %table.products{ 'data-column-preferences-target': "table" }
     %colgroup
-      -# The `min-width` property works in Chrome but not Firefox.
+      -# The `min-width` property works in Chrome but not Firefox so is considered progressive enhancement.
       %col{ 'data-column-preferences-name': :image,      width:"56px" }= # (image size + padding)
-      %col{ 'data-column-preferences-name': :name,       width:"15%", style:"min-width: 6em" }= #  (grow to fill)
-      %col{ 'data-column-preferences-name': :sku,        width:"10%", style:"min-width: 6em" }
+      %col{ 'data-column-preferences-name': :name,       style:"min-width: 6em" }= # (grow to fill)
+      %col{ 'data-column-preferences-name': :sku,        width:"8%", style:"min-width: 6em" }
       %col{ 'data-column-preferences-name': :unit_scale, width:"8%" }
       %col{ 'data-column-preferences-name': :unit,       width:"8%" }
       %col{ 'data-column-preferences-name': :price,      width:"5%", style:"min-width: 5em" }
       %col{ 'data-column-preferences-name': :on_hand,    width:"10%"}
-      %col{ 'data-column-preferences-name': :producer,   width:"15%"}
+      %col{ 'data-column-preferences-name': :producer,   style:"min-width: 6em" }= # (grow to fill)
       %col{ 'data-column-preferences-name': :category,   width:"8%" }
       %col{ 'data-column-preferences-name': :tax_category, width:"8%" }
-      %col{ 'data-column-preferences-name': :inherits_properties, width:"8%" }
-      %col{ width:"1%", style:"min-width: 5em"}= # Actions
+      %col{ 'data-column-preferences-name': :inherits_properties, width:"5%" }
+      %col{ width:"5%", style:"min-width: 3em"}= # Actions
     %thead
       %tr
         %td.form-actions-wrapper{ colspan: 12 }
diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb
index 511b7885e2..620e1901a2 100644
--- a/spec/system/admin/products_v3/products_spec.rb
+++ b/spec/system/admin/products_v3/products_spec.rb
@@ -1038,7 +1038,6 @@ RSpec.describe 'As an enterprise user, I can manage my products', feature: :admi
           tomselect_search_and_select(category_to_select, from: "Category")
 
           sleep(0.1)
-          pending "unable to select tax_category dropdown, possibly because category is still open and too big, covering the tax_cat." # rubocop:disable Layout/LineLength
           validate_tomselect_with_search!(
             page, "Tax Category",
             tax_categories_search_selector

From b8f8d6d042148a38b10d9be1a035889e034c4ac4 Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Tue, 18 Jun 2024 15:28:49 +1000
Subject: [PATCH 24/25] Show/hide columns with CSS selectors instead

Well, that made the JS way simpler.

Adds a lot of classes though. Maybe we could do it based on column index instead, but this will do for now.
table.hide-col0 { td:nth-child(0) { display: none; } }
---
 .../admin/products_v3/_product_row.html.haml  | 22 +++++-----
 app/views/admin/products_v3/_table.html.haml  | 42 +++++++++----------
 .../admin/products_v3/_variant_row.html.haml  | 22 +++++-----
 .../_stimulus_sortable_header.html.haml       |  2 +-
 .../column_preferences_controller.js          | 29 ++-----------
 app/webpacker/css/admin/products_v3.scss      | 19 +++++++--
 6 files changed, 63 insertions(+), 73 deletions(-)

diff --git a/app/views/admin/products_v3/_product_row.html.haml b/app/views/admin/products_v3/_product_row.html.haml
index d009b544ed..c918937570 100644
--- a/app/views/admin/products_v3/_product_row.html.haml
+++ b/app/views/admin/products_v3/_product_row.html.haml
@@ -1,13 +1,13 @@
-%td.with-image{ id: "image-#{product.id}" }
+%td.col-image.with-image{ id: "image-#{product.id}" }
   = render partial: "product_image", locals: { product: }
-%td.field.align-left.header.naked_inputs
+%td.col-name.field.align-left.header.naked_inputs
   = f.hidden_field :id
   = f.text_field :name, 'aria-label': t('admin.products_page.columns.name')
   = error_message_on product, :name
-%td.field.naked_inputs
+%td.col-sku.field.naked_inputs
   = f.text_field :sku, 'aria-label': t('admin.products_page.columns.sku')
   = error_message_on product, :sku
-%td.field.naked_inputs{ 'data-controller': 'toggle-control', 'data-toggle-control-match-value': 'items' }
+%td.col-unit_scale.field.naked_inputs{ 'data-controller': 'toggle-control', 'data-toggle-control-match-value': 'items' }
   = f.hidden_field :variant_unit
   = f.hidden_field :variant_unit_scale
   = f.select :variant_unit_with_scale,
@@ -19,23 +19,23 @@
   .field
     = f.text_field :variant_unit_name, 'aria-label': t('items'), 'data-toggle-control-target': 'control', style: (product.variant_unit == "items" ? "" : "display: none")
     = error_message_on product, :variant_unit_name, 'data-toggle-control-target': 'control'
-%td.align-right
+%td.col-unit.align-right
   -# empty
-%td.align-right
+%td.col-price.align-right
   -# empty
-%td.align-right
+%td.col-on_demand.align-right
   -# empty
-%td.naked_inputs
+%td.col-producer.naked_inputs
   = render(SearchableDropdownComponent.new(form: f,
       name: :supplier_id,
       aria_label: t('.producer_field_name'),
       options: producer_options,
       selected_option: product.supplier_id,
       placeholder_value: t('admin.products_v3.filters.search_for_producers')))
-%td.align-left
+%td.col-category.align-left
   -# empty
-%td.align-left
-%td.align-left
+%td.col-tax_category.align-left
+%td.col-inherits_properties.align-left
   .content= product.inherits_properties ? 'YES' : 'NO' #TODO: consider using https://github.com/RST-J/human_attribute_values, else use I18n.t (also below)
 %td.align-right
   = render(VerticalEllipsisMenu::Component.new) do
diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml
index d15e471012..0b8cc98b79 100644
--- a/app/views/admin/products_v3/_table.html.haml
+++ b/app/views/admin/products_v3/_table.html.haml
@@ -16,17 +16,17 @@
   %table.products{ 'data-column-preferences-target': "table" }
     %colgroup
       -# The `min-width` property works in Chrome but not Firefox so is considered progressive enhancement.
-      %col{ 'data-column-preferences-name': :image,      width:"56px" }= # (image size + padding)
-      %col{ 'data-column-preferences-name': :name,       style:"min-width: 6em" }= # (grow to fill)
-      %col{ 'data-column-preferences-name': :sku,        width:"8%", style:"min-width: 6em" }
-      %col{ 'data-column-preferences-name': :unit_scale, width:"8%" }
-      %col{ 'data-column-preferences-name': :unit,       width:"8%" }
-      %col{ 'data-column-preferences-name': :price,      width:"5%", style:"min-width: 5em" }
-      %col{ 'data-column-preferences-name': :on_hand,    width:"10%"}
-      %col{ 'data-column-preferences-name': :producer,   style:"min-width: 6em" }= # (grow to fill)
-      %col{ 'data-column-preferences-name': :category,   width:"8%" }
-      %col{ 'data-column-preferences-name': :tax_category, width:"8%" }
-      %col{ 'data-column-preferences-name': :inherits_properties, width:"5%" }
+      %col.col-image{ width:"56px" }= # (image size + padding)
+      %col.col-name{ style:"min-width: 6em" }= # (grow to fill)
+      %col.col-sku{ width:"8%", style:"min-width: 6em" }
+      %col.col-unit_scale{ width:"8%" }
+      %col.col-unit{ width:"8%" }
+      %col.col-price{ width:"5%", style:"min-width: 5em" }
+      %col.col-on_hand{ width:"10%"}
+      %col.col-producer{ style:"min-width: 6em" }= # (grow to fill)
+      %col.col-category{ width:"8%" }
+      %col.col-tax_category{ width:"8%" }
+      %col.col-inherits_properties{ width:"5%" }
       %col{ width:"5%", style:"min-width: 3em"}= # Actions
     %thead
       %tr
@@ -49,18 +49,18 @@
                     = t('.reset')
                   = form.submit t('.save'), class: "medium"
       %tr
-        %th.align-left= # image
+        %th.col-image.align-left= # image
         = render partial: 'spree/admin/shared/stimulus_sortable_header',
             locals: { column: :name, sorted: params.dig(:q, :s), default: 'name asc' }
-        %th.align-left.with-input= t('admin.products_page.columns.sku')
-        %th.align-left.with-input= t('admin.products_page.columns.unit_scale')
-        %th.align-left.with-input= t('admin.products_page.columns.unit')
-        %th.align-left.with-input= t('admin.products_page.columns.price')
-        %th.align-left.with-input= t('admin.products_page.columns.on_hand')
-        %th.align-left= t('admin.products_page.columns.producer')
-        %th.align-left= t('admin.products_page.columns.category')
-        %th.align-left= t('admin.products_page.columns.tax_category')
-        %th.align-left= t('admin.products_page.columns.inherits_properties')
+        %th.align-left.col-sku.with-input= t('admin.products_page.columns.sku')
+        %th.align-left.col-unit_scale.with-input= t('admin.products_page.columns.unit_scale')
+        %th.align-left.col-unit.with-input= t('admin.products_page.columns.unit')
+        %th.align-left.col-price.with-input= t('admin.products_page.columns.price')
+        %th.align-left.col-on_hand.with-input= t('admin.products_page.columns.on_hand')
+        %th.align-left.col-producer= t('admin.products_page.columns.producer')
+        %th.align-left.col-category= t('admin.products_page.columns.category')
+        %th.align-left.col-tax_category= t('admin.products_page.columns.tax_category')
+        %th.align-left.col-inherits_properties= t('admin.products_page.columns.inherits_properties')
         %th.align-right= t('admin.products_page.columns.actions')
     - products.each_with_index do |product, product_index|
       = form.fields_for("products", product, index: product_index) do |product_form|
diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml
index 3b3b38ac54..311b833e04 100644
--- a/app/views/admin/products_v3/_variant_row.html.haml
+++ b/app/views/admin/products_v3/_variant_row.html.haml
@@ -1,15 +1,15 @@
-%td
+%td.col-image
   -# empty
-%td.field.naked_inputs
+%td.col-name.field.naked_inputs
   = f.hidden_field :id
   = f.text_field :display_name, 'aria-label': t('admin.products_page.columns.name'), placeholder: variant.product.name
   = error_message_on variant, :display_name
-%td.field.naked_inputs
+%td.col-sku.field.naked_inputs
   = f.text_field :sku, 'aria-label': t('admin.products_page.columns.sku')
   = error_message_on variant, :sku
-%td
+%td.col-unit_scale
   -# empty
-%td.field.popout{'data-controller': "popout", 'data-popout-update-display-value': "false"}
+%td.col-unit.field.popout{'data-controller': "popout", 'data-popout-update-display-value': "false"}
   = f.button :unit_to_display, class: "popout__button", 'aria-label': t('admin.products_page.columns.unit'), 'data-popout-target': "button" do
     = variant.unit_to_display # Show the generated summary of unit values
   %div.popout__container{ style: 'display: none;', 'data-controller': 'toggle-control', 'data-popout-target': "dialog" }
@@ -25,10 +25,10 @@
       = f.label :display_as, t('admin.products_page.columns.display_as')
       = f.text_field :display_as, placeholder: VariantUnits::OptionValueNamer.new(variant).name
   = error_message_on variant, :unit_value
-%td.field.naked_inputs
+%td.col-price.field.naked_inputs
   = f.text_field :price, 'aria-label': t('admin.products_page.columns.price'), value: number_to_currency(variant.price, unit: '')&.strip # TODO: add a spec to prove that this formatting is necessary. If so, it should be in a shared form helper for currency inputs
   = error_message_on variant, :price
-%td.field.popout{'data-controller': "popout"}
+%td.col-on_hand.field.popout{'data-controller': "popout"}
   %button.popout__button{'data-popout-target': "button", 'aria-label': t('admin.products_page.columns.on_hand')}
     = variant.on_demand ? t(:on_demand) : variant.on_hand
   %div.popout__container{ style: 'display: none;', 'data-controller': 'toggle-control', 'data-popout-target': "dialog" }
@@ -39,16 +39,16 @@
       = f.label :on_demand do
         = f.check_box :on_demand, 'data-action': 'change->toggle-control#disableIfPresent change->popout#closeIfChecked'
         = t(:on_demand)
-%td.align-left
+%td.col-producer.align-left
   -# empty producer name
-%td.field.naked_inputs
+%td.col-category.field.naked_inputs
   = render(SearchableDropdownComponent.new(form: f,
       name: :primary_taxon_id,
       options: category_options,
       selected_option: variant.primary_taxon_id,
       aria_label: t('.category_field_name'),
       placeholder_value: t('admin.products_v3.filters.search_for_categories')))
-%td.field.naked_inputs
+%td.col-tax_category.field.naked_inputs
   = render(SearchableDropdownComponent.new(form: f,
       name: :tax_category_id,
       options: tax_category_options,
@@ -57,7 +57,7 @@
       aria_label: t('.tax_category_field_name'),
       placeholder_value: t('.search_for_tax_categories')))
   = error_message_on variant, :tax_category
-%td.align-left
+%td.col-inherits_properties.align-left
   -# empty
 %td.align-right
   = render(VerticalEllipsisMenu::Component.new) do
diff --git a/app/views/spree/admin/shared/_stimulus_sortable_header.html.haml b/app/views/spree/admin/shared/_stimulus_sortable_header.html.haml
index 08db1b6ce9..051ffa1f4f 100644
--- a/app/views/spree/admin/shared/_stimulus_sortable_header.html.haml
+++ b/app/views/spree/admin/shared/_stimulus_sortable_header.html.haml
@@ -1,4 +1,4 @@
-%th
+%th{ class: "col-#{column}" }
   %a{ "data-controller": "search", "data-action": "click->search#changeSorting", "data-column": "#{column}", "data-current": (sorted || default).to_s }
     = t("spree.admin.shared.sortable_header.#{column.to_s}")
 
diff --git a/app/webpacker/controllers/column_preferences_controller.js b/app/webpacker/controllers/column_preferences_controller.js
index dfbcee6f2b..9b55a961dc 100644
--- a/app/webpacker/controllers/column_preferences_controller.js
+++ b/app/webpacker/controllers/column_preferences_controller.js
@@ -12,7 +12,7 @@ export default class ColumnPreferencesController extends Controller {
       cell.dataset.defaultColSpan ||= cell.colSpan;
     });
 
-    this.checkboxes = this.element.querySelectorAll("input[type=checkbox]");
+    this.checkboxes = Array.from(this.element.querySelectorAll("input[type=checkbox]"));
     for (const element of this.checkboxes) {
       // On initial load
       this.#showHideColumn(element);
@@ -26,40 +26,17 @@ export default class ColumnPreferencesController extends Controller {
   #showHideColumn(e) {
     const element = e.target || e;
     const name = element.dataset.columnName;
-    const selector = `col[data-column-preferences-name="${name}"]`;
-    const column = this.table.querySelector(selector);
-    const index = this.#getIndex(column);
 
-    if (column == null) {
-      console.error(`ColumnPreferencesController: could not find ${selector}`);
-      return;
-    }
-
-    // Hide column definition
-    this.#showHideElement(column, element.checked);
-
-    // Hide each cell in column (ignore rows with colspan)
-    const rows = this.table.querySelectorAll("tr:not(:has(td[colspan]))");
-    rows.forEach((row) => {
-      // Ignore cell if spanning multiple columns
-      const cell = row.children[index];
-      if (cell == undefined) return;
-
-      this.#showHideElement(cell, element.checked);
-    });
+    this.table.classList.toggle(`hide-${name}`, !element.checked);
 
     // Reset cell colspans
-    const hiddenColCount = this.cols.filter((col)=> col.style.display == 'none').length;
+    const hiddenColCount = this.checkboxes.filter((checkbox)=> !checkbox.checked).length;
     for(const cell of this.colSpanCells) {
       const span = parseInt(cell.dataset.defaultColSpan, 10) - hiddenColCount;
       cell.colSpan = span;
     };
   }
 
-  #getIndex(column) {
-    return Array.from(column.parentNode.children).indexOf(column);
-  }
-
   #showHideElement(element, show) {
     element.style.display = show ? "" : "none";
   }
diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss
index 646c2d074b..576e6d8af1 100644
--- a/app/webpacker/css/admin/products_v3.scss
+++ b/app/webpacker/css/admin/products_v3.scss
@@ -96,7 +96,8 @@
 
       // "Naked" inputs. Row hover helps reveal them.
       .naked_inputs {
-        input:not([type="checkbox"]), .ts-control {
+        input:not([type="checkbox"]),
+        .ts-control {
           background-color: $color-tbl-cell-bg;
         }
       }
@@ -179,6 +180,18 @@
     .ts-control {
       z-index: 0; // Avoid hovering over thead
     }
+
+    // Hide columns
+    $columns:
+      "image", "name", "sku", "unit_scale", "unit", "price", "on_hand", "producer", "category",
+      "tax_category", "inherits_properties";
+    @each $col in $columns {
+      &.hide-#{$col} {
+        .col-#{$col} {
+          display: none;
+        }
+      }
+    }
   }
 
   #no-products {
@@ -378,7 +391,7 @@
       border-radius: $border-radius;
       box-shadow: 0px 0px 8px 0px rgba($near-black, 0.25);
 
-      .field{
+      .field {
         margin-bottom: 0.75em;
 
         &:last-child {
@@ -432,7 +445,7 @@
       opacity: 0;
     }
   }
-  
+
   .slide-out {
     animation: slideOutLeft 0.5s forwards;
   }

From 6fd3cada8cb563d1c83d874e92394d018ca4261a Mon Sep 17 00:00:00 2001
From: David Cook <david@redcliffs.net>
Date: Wed, 19 Jun 2024 09:13:53 +1000
Subject: [PATCH 25/25] Fix classname

---
 app/views/admin/products_v3/_product_row.html.haml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/views/admin/products_v3/_product_row.html.haml b/app/views/admin/products_v3/_product_row.html.haml
index c918937570..48636dc92e 100644
--- a/app/views/admin/products_v3/_product_row.html.haml
+++ b/app/views/admin/products_v3/_product_row.html.haml
@@ -23,7 +23,7 @@
   -# empty
 %td.col-price.align-right
   -# empty
-%td.col-on_demand.align-right
+%td.col-on_hand.align-right
   -# empty
 %td.col-producer.naked_inputs
   = render(SearchableDropdownComponent.new(form: f,