From d3b653b257083edf05a53947465f954fd513cb72 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 11 Mar 2025 11:09:28 +1100
Subject: [PATCH 01/24] Add "variant_tag" to variant

---
 app/models/spree/variant.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb
index 7cd8e1a10b..20c60a9a5c 100644
--- a/app/models/spree/variant.rb
+++ b/app/models/spree/variant.rb
@@ -18,6 +18,8 @@ module Spree
 
     acts_as_paranoid
 
+    acts_as_taggable_on :variant_tag
+
     searchable_attributes :sku, :display_as, :display_name, :primary_taxon_id, :supplier_id
     searchable_associations :product, :default_price, :primary_taxon, :supplier
     searchable_scopes :active, :deleted

From 00db5091e73d6cf10de71974cefe670e8385e9ee Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 11 Mar 2025 15:01:23 +1100
Subject: [PATCH 02/24] Add "variant_tag" feature toggle

---
 lib/open_food_network/feature_toggle.rb | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/open_food_network/feature_toggle.rb b/lib/open_food_network/feature_toggle.rb
index 13c07573b2..524a273f0b 100644
--- a/lib/open_food_network/feature_toggle.rb
+++ b/lib/open_food_network/feature_toggle.rb
@@ -61,6 +61,9 @@ module OpenFoodNetwork
       "open_in_same_tab" => <<~DESC,
         Open the admin dashboard in the same tab instead of a new tab.
       DESC
+      "variant_tag" => <<~DESC,
+        Variant Tag are available on the Bulk Edit Products page.
+      DESC
     }.merge(conditional_features).freeze;
 
     # Features you would like to be enabled to start with.

From 87a80d3efe7ddf90d03698254fc000c0a489e12d Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 11 Mar 2025 16:18:22 +1100
Subject: [PATCH 03/24] Add Tags collumn

Currently display the list of tags, no fancy UI.
Updating works
---
 app/services/permitted_attributes/variant.rb           |  2 +-
 app/views/admin/products_v3/_product_row.html.haml     |  3 +++
 .../admin/products_v3/_product_variant_row.html.haml   |  3 ++-
 app/views/admin/products_v3/_table.html.haml           |  7 ++++++-
 app/views/admin/products_v3/_variant_row.html.haml     |  3 +++
 .../controllers/column_preferences_controller.js       |  1 +
 app/webpacker/css/admin/products_v3.scss               |  2 +-
 config/locales/en.yml                                  |  1 +
 lib/open_food_network/column_preference_defaults.rb    | 10 ++++++++--
 9 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/app/services/permitted_attributes/variant.rb b/app/services/permitted_attributes/variant.rb
index e333eded25..a538092699 100644
--- a/app/services/permitted_attributes/variant.rb
+++ b/app/services/permitted_attributes/variant.rb
@@ -8,7 +8,7 @@ module PermittedAttributes
         :shipping_category_id, :price, :unit_value,
         :unit_description, :variant_unit, :variant_unit_name, :variant_unit_scale, :display_name,
         :display_as, :tax_category_id, :weight, :height, :width, :depth, :taxon_ids,
-        :primary_taxon_id, :supplier_id
+        :primary_taxon_id, :supplier_id, :variant_tag_list
       ]
     end
   end
diff --git a/app/views/admin/products_v3/_product_row.html.haml b/app/views/admin/products_v3/_product_row.html.haml
index 707a7192ba..980b9980d8 100644
--- a/app/views/admin/products_v3/_product_row.html.haml
+++ b/app/views/admin/products_v3/_product_row.html.haml
@@ -18,6 +18,9 @@
 %td.col-category.align-left
   -# empty
 %td.col-tax_category.align-left
+- if feature?(:variant_tag, spree_current_user)
+  %td.col-tags.align-left
+    -# empty
 %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
diff --git a/app/views/admin/products_v3/_product_variant_row.html.haml b/app/views/admin/products_v3/_product_variant_row.html.haml
index 12b6702cba..f5144fe7d7 100644
--- a/app/views/admin/products_v3/_product_variant_row.html.haml
+++ b/app/views/admin/products_v3/_product_variant_row.html.haml
@@ -19,7 +19,8 @@
     %tr{ 'data-nested-form-target': "target" }
     %tr.condensed
       %td
-      %td{ colspan: 11 }
+      - colspan = feature?(:variant_tag, spree_current_user) ? 12 : 11
+      %td{ colspan: "#{colspan}" }
         %button.secondary.condensed.naked.icon-plus{ 'data-action': "nested-form#add",
                                                       'aria-label': t('.new_variant') }
           =t('.new_variant')
diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml
index 5f30291408..5226461c94 100644
--- a/app/views/admin/products_v3/_table.html.haml
+++ b/app/views/admin/products_v3/_table.html.haml
@@ -26,11 +26,14 @@
       %col.col-producer{ style:"min-width: 6em" }= # (grow to fill)
       %col.col-category{ width:"8%" }
       %col.col-tax_category{ width:"8%" }
+      - if feature?(:variant_tag, spree_current_user)
+        %col.col-tags{ width:"8%" }
       %col.col-inherits_properties{ width:"5%" }
       %col{ width:"5%", style:"min-width: 3em"}= # Actions
     %thead
       %tr
-        %td.form-actions-wrapper{ colspan: 12 }
+        - colspan = feature?(:variant_tag, spree_current_user) ? 13 : 12
+        %td.form-actions-wrapper{ colspan: "#{colspan}" }
           .form-actions-wrapper2
             %fieldset.form-actions{ class: ("hidden" unless defined?(@error_counts)), 'data-bulk-form-target': "actions" }
               .container
@@ -60,6 +63,8 @@
         %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')
+        - if feature?(:variant_tag, spree_current_user)
+          %th.align-left.col-tags= t('admin.products_page.columns.tags')
         %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|
diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml
index d87763e42f..e347cb1d07 100644
--- a/app/views/admin/products_v3/_variant_row.html.haml
+++ b/app/views/admin/products_v3/_variant_row.html.haml
@@ -74,6 +74,9 @@
       aria_label: t('.tax_category_field_name'),
       placeholder_value: t('.search_for_tax_categories')))
   = error_message_on variant, :tax_category
+- if feature?(:variant_tag, spree_current_user)
+  %td.col-tags.field.naked_inputs
+    = f.text_field :variant_tag_list, 'aria-label': t('admin.products_page.columns.tags'), value: variant.variant_tag_list.join(",")
 %td.col-inherits_properties.align-left
   -# empty
 %td.align-right
diff --git a/app/webpacker/controllers/column_preferences_controller.js b/app/webpacker/controllers/column_preferences_controller.js
index 65202920f4..1d314fc4be 100644
--- a/app/webpacker/controllers/column_preferences_controller.js
+++ b/app/webpacker/controllers/column_preferences_controller.js
@@ -29,6 +29,7 @@ export default class ColumnPreferencesController extends Controller {
     const element = e.target || e;
     const name = element.dataset.columnName;
 
+    // Css defined in app/webpacker/css/admin/products_v3.scss
     this.table.classList.toggle(`hide-${name}`, !element.checked);
 
     // Reset cell colspans
diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss
index bb144cd6e8..4dd6331775 100644
--- a/app/webpacker/css/admin/products_v3.scss
+++ b/app/webpacker/css/admin/products_v3.scss
@@ -186,7 +186,7 @@
     // Hide columns
     $columns:
       "image", "name", "sku", "unit_scale", "unit", "price", "on_hand", "producer", "category",
-      "tax_category", "inherits_properties";
+      "tax_category", "tags", "inherits_properties";
     @each $col in $columns {
       &.hide-#{$col} {
         .col-#{$col} {
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 7945ba1a25..9b735dca1d 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -636,6 +636,7 @@ en:
         inherits_properties: "Inherits Properties?"
         import_date: "Import Date"
         actions: Actions
+        tags: Tags 
       columns_selector:
         unit: Unit
         price: Price
diff --git a/lib/open_food_network/column_preference_defaults.rb b/lib/open_food_network/column_preference_defaults.rb
index 9da9ef36e9..9eca6be307 100644
--- a/lib/open_food_network/column_preference_defaults.rb
+++ b/lib/open_food_network/column_preference_defaults.rb
@@ -81,7 +81,7 @@ module OpenFoodNetwork
       producer_visibility = display_producer_column?(user)
 
       I18n.with_options scope: 'admin.products_page.columns' do
-        {
+        columns = {
           image: { name: t(:image), visible: true },
           name: { name: t(:name), visible: true },
           sku: { name: t(:sku), visible: true },
@@ -92,8 +92,14 @@ module OpenFoodNetwork
           producer: { name: t(:producer), visible: producer_visibility },
           category: { name: t(:category), visible: true },
           tax_category: { name: t(:tax_category), visible: true },
-          inherits_properties: { name: t(:inherits_properties), visible: true },
         }
+        if OpenFoodNetwork::FeatureToggle.enabled?(:variant_tag, user)
+          columns[:tags] = { name: t(:tags), visible: true }
+        end
+
+        columns[:inherits_properties] = { name: t(:inherits_properties), visible: true }
+
+        columns
       end
     end
 

From 352203747a171f1c60c79f52b852ccc7fd1415ca Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 12 Mar 2025 11:56:00 +1100
Subject: [PATCH 04/24] Add tag styling

---
 .../admin/products_v3/_variant_row.html.haml  | 12 ++-
 app/webpacker/css/admin/products_v3.scss      | 79 ++++++++++++++++++-
 config/locales/en.yml                         |  1 +
 3 files changed, 88 insertions(+), 4 deletions(-)

diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml
index e347cb1d07..c1e22b2b3a 100644
--- a/app/views/admin/products_v3/_variant_row.html.haml
+++ b/app/views/admin/products_v3/_variant_row.html.haml
@@ -76,7 +76,17 @@
   = error_message_on variant, :tax_category
 - if feature?(:variant_tag, spree_current_user)
   %td.col-tags.field.naked_inputs
-    = f.text_field :variant_tag_list, 'aria-label': t('admin.products_page.columns.tags'), value: variant.variant_tag_list.join(",")
+    = f.hidden_field :variant_tag_list, value: variant.variant_tag_list.join(",")
+    .tags-input
+      .tags
+        %ul.tag-list
+          - variant.variant_tag_list.each do |tag|
+            %li.tag-item
+              .tag-template
+                %span=tag
+                %a.remove-button
+                  ✖
+        = text_field_tag :variant_add_tag, nil, class: "input", placeholder: t('.add_a_tag')
 %td.col-inherits_properties.align-left
   -# empty
 %td.align-right
diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss
index 4dd6331775..c7dc169c7d 100644
--- a/app/webpacker/css/admin/products_v3.scss
+++ b/app/webpacker/css/admin/products_v3.scss
@@ -184,9 +184,8 @@
     }
 
     // Hide columns
-    $columns:
-      "image", "name", "sku", "unit_scale", "unit", "price", "on_hand", "producer", "category",
-      "tax_category", "tags", "inherits_properties";
+    $columns: "image", "name", "sku", "unit_scale", "unit", "price", "on_hand", "producer",
+      "category", "tax_category", "tags", "inherits_properties";
     @each $col in $columns {
       &.hide-#{$col} {
         .col-#{$col} {
@@ -369,4 +368,78 @@
     transform-origin: top;
     animation: slideInTop 0.5s forwards;
   }
+
+  // Tags input
+  .tags-input {
+    display: block;
+
+    .tags {
+      -moz-appearance: textfield;
+      -webkit-appearance: textfield;
+      overflow: hidden;
+      word-wrap: break-word;
+      cursor: text;
+      background-color: #fff;
+      height: 100%;
+      box-shadow: none;
+
+      .tag-list {
+        margin: 0;
+        padding: 0;
+        list-style-type: none;
+      }
+
+      li.tag-item {
+        -webkit-border-radius: 3px;
+        -moz-border-radius: 3px;
+        -ms-border-radius: 3px;
+        -o-border-radius: 3px;
+        border-radius: 3px;
+        margin: 2px 0 2px 3px;
+        padding: 0 5px;
+        display: inline-block;
+        float: left;
+        font:
+          14px "Helvetica Neue",
+          Helvetica,
+          Arial,
+          sans-serif;
+        font-size: 85%;
+        height: 25px;
+        line-height: 25px;
+        border: none;
+        box-shadow: none;
+        color: white !important;
+        background-image: none;
+        background-color: $teal;
+
+        .remove-button {
+          margin: 0 0 0 5px;
+          padding: 0;
+          border: none;
+          background: 0 0;
+          cursor: pointer;
+          vertical-align: middle;
+          font:
+            700 16px Arial,
+            sans-serif;
+          color: white;
+        }
+      }
+
+      .input {
+        border: 0;
+        outline: 0;
+        margin: 2px;
+        padding: 0 0 0 5px;
+        float: left;
+        height: 26px;
+        font:
+          14px "Helvetica Neue",
+          Helvetica,
+          Arial,
+          sans-serif;
+      }
+    }
+  }
 }
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 9b735dca1d..adbae5c6c2 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -1017,6 +1017,7 @@ en:
         tax_category_field_name: "Tax Category"
         producer_field_name: "Producer"
         select_unit_scale: Select unit scale
+        add_a_tag: Add a tag
       clone:
         success: Successfully cloned the product
         error: Unable to clone the product

From 3585499fbadc3efa249013eabec44d6e267ec49a Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 12 Mar 2025 14:21:16 +1100
Subject: [PATCH 05/24] Move tag list input to a component

---
 app/components/tag_list_input_component.rb    | 13 ++++
 .../tag_list_input_component.html.haml        | 11 +++
 .../tag_list_input_component.scss             | 73 ++++++++++++++++++
 .../admin/products_v3/_variant_row.html.haml  | 12 +--
 app/webpacker/css/admin/products_v3.scss      | 74 -------------------
 app/webpacker/css/admin_v3/all.scss           |  1 +
 6 files changed, 99 insertions(+), 85 deletions(-)
 create mode 100644 app/components/tag_list_input_component.rb
 create mode 100644 app/components/tag_list_input_component/tag_list_input_component.html.haml
 create mode 100644 app/components/tag_list_input_component/tag_list_input_component.scss

diff --git a/app/components/tag_list_input_component.rb b/app/components/tag_list_input_component.rb
new file mode 100644
index 0000000000..0897da332d
--- /dev/null
+++ b/app/components/tag_list_input_component.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class TagListInputComponent < ViewComponent::Base
+  # method in a "hidden_field" form helper and is the method used to get a list of tag on the model
+  def initialize(form:, method:, tags:, placeholder: "Add a tag")
+    @f = form
+    @method = method
+    @tags = tags
+    @placeholder = placeholder
+  end
+
+  attr_reader :f, :method, :tags, :placeholder
+end
diff --git a/app/components/tag_list_input_component/tag_list_input_component.html.haml b/app/components/tag_list_input_component/tag_list_input_component.html.haml
new file mode 100644
index 0000000000..8be197702d
--- /dev/null
+++ b/app/components/tag_list_input_component/tag_list_input_component.html.haml
@@ -0,0 +1,11 @@
+= f.hidden_field method.to_sym, value: tags.join(",")
+.tags-input
+  .tags
+    %ul.tag-list
+      - tags.each do |tag|
+        %li.tag-item
+          .tag-template
+            %span=tag
+            %a.remove-button
+              ✖
+    = text_field_tag :variant_add_tag, nil, class: "input", placeholder: placeholder
diff --git a/app/components/tag_list_input_component/tag_list_input_component.scss b/app/components/tag_list_input_component/tag_list_input_component.scss
new file mode 100644
index 0000000000..751a2a4820
--- /dev/null
+++ b/app/components/tag_list_input_component/tag_list_input_component.scss
@@ -0,0 +1,73 @@
+// Tags input
+.tags-input {
+  display: block;
+
+  .tags {
+    -moz-appearance: textfield;
+    -webkit-appearance: textfield;
+    overflow: hidden;
+    word-wrap: break-word;
+    cursor: text;
+    background-color: #fff;
+    height: 100%;
+    box-shadow: none;
+
+    .tag-list {
+      margin: 0;
+      padding: 0;
+      list-style-type: none;
+    }
+
+    li.tag-item {
+      -webkit-border-radius: 3px;
+      -moz-border-radius: 3px;
+      -ms-border-radius: 3px;
+      -o-border-radius: 3px;
+      border-radius: 3px;
+      margin: 2px 0 2px 3px;
+      padding: 0 5px;
+      display: inline-block;
+      float: left;
+      font:
+        14px "Helvetica Neue",
+        Helvetica,
+        Arial,
+        sans-serif;
+      font-size: 85%;
+      height: 25px;
+      line-height: 25px;
+      border: none;
+      box-shadow: none;
+      color: white !important;
+      background-image: none;
+      background-color: $teal;
+
+      .remove-button {
+        margin: 0 0 0 5px;
+        padding: 0;
+        border: none;
+        background: 0 0;
+        cursor: pointer;
+        vertical-align: middle;
+        font:
+          700 16px Arial,
+          sans-serif;
+        color: white;
+      }
+    }
+
+    .input {
+      border: 0;
+      outline: 0;
+      margin: 2px;
+      padding: 0 0 0 5px;
+      float: left;
+      height: 26px;
+      font:
+        14px "Helvetica Neue",
+        Helvetica,
+        Arial,
+        sans-serif;
+    }
+  }
+}
diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml
index c1e22b2b3a..20ed7f8050 100644
--- a/app/views/admin/products_v3/_variant_row.html.haml
+++ b/app/views/admin/products_v3/_variant_row.html.haml
@@ -76,17 +76,7 @@
   = error_message_on variant, :tax_category
 - if feature?(:variant_tag, spree_current_user)
   %td.col-tags.field.naked_inputs
-    = f.hidden_field :variant_tag_list, value: variant.variant_tag_list.join(",")
-    .tags-input
-      .tags
-        %ul.tag-list
-          - variant.variant_tag_list.each do |tag|
-            %li.tag-item
-              .tag-template
-                %span=tag
-                %a.remove-button
-                  ✖
-        = text_field_tag :variant_add_tag, nil, class: "input", placeholder: t('.add_a_tag')
+    = render TagListInputComponent.new(form: f, method: "variant_tag_list", tags: variant.variant_tag_list, placeholder: t('.add_a_tag'))
 %td.col-inherits_properties.align-left
   -# empty
 %td.align-right
diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss
index c7dc169c7d..9944f515af 100644
--- a/app/webpacker/css/admin/products_v3.scss
+++ b/app/webpacker/css/admin/products_v3.scss
@@ -368,78 +368,4 @@
     transform-origin: top;
     animation: slideInTop 0.5s forwards;
   }
-
-  // Tags input
-  .tags-input {
-    display: block;
-
-    .tags {
-      -moz-appearance: textfield;
-      -webkit-appearance: textfield;
-      overflow: hidden;
-      word-wrap: break-word;
-      cursor: text;
-      background-color: #fff;
-      height: 100%;
-      box-shadow: none;
-
-      .tag-list {
-        margin: 0;
-        padding: 0;
-        list-style-type: none;
-      }
-
-      li.tag-item {
-        -webkit-border-radius: 3px;
-        -moz-border-radius: 3px;
-        -ms-border-radius: 3px;
-        -o-border-radius: 3px;
-        border-radius: 3px;
-        margin: 2px 0 2px 3px;
-        padding: 0 5px;
-        display: inline-block;
-        float: left;
-        font:
-          14px "Helvetica Neue",
-          Helvetica,
-          Arial,
-          sans-serif;
-        font-size: 85%;
-        height: 25px;
-        line-height: 25px;
-        border: none;
-        box-shadow: none;
-        color: white !important;
-        background-image: none;
-        background-color: $teal;
-
-        .remove-button {
-          margin: 0 0 0 5px;
-          padding: 0;
-          border: none;
-          background: 0 0;
-          cursor: pointer;
-          vertical-align: middle;
-          font:
-            700 16px Arial,
-            sans-serif;
-          color: white;
-        }
-      }
-
-      .input {
-        border: 0;
-        outline: 0;
-        margin: 2px;
-        padding: 0 0 0 5px;
-        float: left;
-        height: 26px;
-        font:
-          14px "Helvetica Neue",
-          Helvetica,
-          Arial,
-          sans-serif;
-      }
-    }
-  }
 }
diff --git a/app/webpacker/css/admin_v3/all.scss b/app/webpacker/css/admin_v3/all.scss
index a339a20457..3e3cc403bd 100644
--- a/app/webpacker/css/admin_v3/all.scss
+++ b/app/webpacker/css/admin_v3/all.scss
@@ -128,6 +128,7 @@
 
 @import "app/components/modal_component/modal_component";
 @import "app/components/vertical_ellipsis_menu/component"; // admin_v3 and only V3
+@import "app/components/tag_list_input_component/tag_list_input_component";
 @import "app/webpacker/css/admin/trix.scss";
 
 @import "terms_of_service_banner"; // admin_v3

From d6ed536eb794d2dd3ecb7249dbb0325bbbd1e6eb Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 12 Mar 2025 15:11:22 +1100
Subject: [PATCH 06/24] Add example of component stimulus controller naming
 convention

---
 app/components/example_component/example_component.html.haml | 4 +++-
 app/components/example_component/example_controller.js       | 4 ++++
 2 files changed, 7 insertions(+), 1 deletion(-)
 create mode 100644 app/components/example_component/example_controller.js

diff --git a/app/components/example_component/example_component.html.haml b/app/components/example_component/example_component.html.haml
index d0ac42c229..c4ec93450a 100644
--- a/app/components/example_component/example_component.html.haml
+++ b/app/components/example_component/example_component.html.haml
@@ -1 +1,3 @@
-%h1 #{@title}
+- # the stimulus contoller "example-component--example" lives in app/component/example_component/example_controller.js
+%div{ "data-controller": "example-component--example"}
+  %h1 #{@title}
diff --git a/app/components/example_component/example_controller.js b/app/components/example_component/example_controller.js
new file mode 100644
index 0000000000..72957edffd
--- /dev/null
+++ b/app/components/example_component/example_controller.js
@@ -0,0 +1,4 @@
+// This controller will be called "example-component--example", ie "component-subdirectory--js-file-name"
+import { Controller } from "stimulus";
+
+export default class extends Controller {}

From 6ff4c2c3aac4cf621eaea3f2e0b90b3762c5427d Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Mon, 7 Apr 2025 14:05:58 +1000
Subject: [PATCH 07/24] Disable submitting form with enter key

We need to the enter key available to create new tags
---
 app/webpacker/controllers/bulk_form_controller.js | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/app/webpacker/controllers/bulk_form_controller.js b/app/webpacker/controllers/bulk_form_controller.js
index 476bf00391..5cffebaee9 100644
--- a/app/webpacker/controllers/bulk_form_controller.js
+++ b/app/webpacker/controllers/bulk_form_controller.js
@@ -23,10 +23,16 @@ export default class BulkFormController extends Controller {
   recordElements = {};
 
   connect() {
+    // disable form submit via enter key, so we can use enter key to create new product tags
+    hotkeys('enter', function (event, handler) {
+      event.preventDefault();
+    });
+
     this.submitting = false;
     this.form = this.element;
 
     // Start listening for any changes within the form
+    // TODO make sure tag inpug appreas here when deleted
     this.#registerElements(this.form.elements);
 
     this.toggleFormChanged();

From c15e16900df2e3bf7839ffdc643f49a88828c2f4 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Mon, 7 Apr 2025 14:12:42 +1000
Subject: [PATCH 08/24] Add stimulus tag list input controller

It handles the UI to display the list of tags, and lets you add and
remove tags from the list.
---
 .../tag_list_input_component.html.haml        |  30 +++--
 .../tag_list_input_component.scss             |   1 -
 .../tag_list_input_controller.js              |  40 ++++++
 .../tag_list_input_controller_test.js         | 126 ++++++++++++++++++
 4 files changed, 185 insertions(+), 12 deletions(-)
 create mode 100644 app/components/tag_list_input_component/tag_list_input_controller.js
 create mode 100644 spec/javascripts/stimulus/tag_list_input_controller_test.js

diff --git a/app/components/tag_list_input_component/tag_list_input_component.html.haml b/app/components/tag_list_input_component/tag_list_input_component.html.haml
index 8be197702d..0149584b2c 100644
--- a/app/components/tag_list_input_component/tag_list_input_component.html.haml
+++ b/app/components/tag_list_input_component/tag_list_input_component.html.haml
@@ -1,11 +1,19 @@
-= f.hidden_field method.to_sym, value: tags.join(",")
-.tags-input
-  .tags
-    %ul.tag-list
-      - tags.each do |tag|
-        %li.tag-item
-          .tag-template
-            %span=tag
-            %a.remove-button
-              ✖
-    = text_field_tag :variant_add_tag, nil, class: "input", placeholder: placeholder
+- #%div{ "data-controller": "tag-list-input-component--tag-list-input" }
+%div{ "data-controller": "tag-list-input-component--tag-list-input" }
+  = f.hidden_field method.to_sym, value: tags.join(","), "data-tag-list-input-component--tag-list-input-target": "tagList"
+  .tags-input{  }
+    .tags
+      %ul.tag-list{"data-tag-list-input-component--tag-list-input-target": "list"}
+        %template{"data-tag-list-input-component--tag-list-input-target": "template"}
+          %li.tag-item
+            .tag-template
+              %span
+              %a.remove-button{ "data-action": "click->tag-list-input-component--tag-list-input#removeTag" }
+                ✖ 
+        - tags.each do |tag|
+          %li.tag-item
+            .tag-template
+              %span=tag
+              %a.remove-button{ "data-action": "click->tag-list-input-component--tag-list-input#removeTag" }
+                ✖
+      = text_field_tag "variant_add_tag_#{f.object.id}".to_sym, nil, class: "input", placeholder: placeholder, "data-action": "keydown.enter->tag-list-input-component--tag-list-input#addTag keyup->tag-list-input-component--tag-list-input#filterInput", "data-tag-list-input-component--tag-list-input-target": "newTag"
diff --git a/app/components/tag_list_input_component/tag_list_input_component.scss b/app/components/tag_list_input_component/tag_list_input_component.scss
index 751a2a4820..64e7998d29 100644
--- a/app/components/tag_list_input_component/tag_list_input_component.scss
+++ b/app/components/tag_list_input_component/tag_list_input_component.scss
@@ -33,7 +33,6 @@
         Helvetica,
         Arial,
         sans-serif;
-      font-size: 85%;
       height: 25px;
       line-height: 25px;
       border: none;
diff --git a/app/components/tag_list_input_component/tag_list_input_controller.js b/app/components/tag_list_input_component/tag_list_input_controller.js
new file mode 100644
index 0000000000..1316b85855
--- /dev/null
+++ b/app/components/tag_list_input_component/tag_list_input_controller.js
@@ -0,0 +1,40 @@
+import { Controller } from "stimulus";
+
+export default class extends Controller {
+  static targets = ["tagList", "newTag", "template", "list"];
+
+  addTag() {
+    // add to tagList
+    this.tagListTarget.value = this.tagListTarget.value.concat(`,${this.newTagTarget.value}`);
+
+    // Create new li component with value
+    const newTagElement = this.templateTarget.content.cloneNode(true);
+    const spanElement = newTagElement.querySelector("span");
+    spanElement.innerText = this.newTagTarget.value;
+    this.listTarget.appendChild(newTagElement);
+
+    // Clear new tag value
+    this.newTagTarget.value = "";
+  }
+
+  removeTag(event) {
+    // Text to remove
+    const tagName = event.srcElement.previousElementSibling.textContent;
+
+    // Remove tag from list
+    const tags = this.tagListTarget.value.split(",");
+    const index = tags.indexOf(tagName);
+    tags.splice(index, 1);
+    this.tagListTarget.value = tags.join(",");
+
+    // Remove HTML element from the list
+    event.srcElement.parentElement.parentElement.remove();
+  }
+
+  // Strip comma from tag name
+  filterInput(event) {
+    if (event.key === ",") {
+      event.srcElement.value = event.srcElement.value.replace(",","");
+    }
+  }
+}
diff --git a/spec/javascripts/stimulus/tag_list_input_controller_test.js b/spec/javascripts/stimulus/tag_list_input_controller_test.js
new file mode 100644
index 0000000000..b9c266de1d
--- /dev/null
+++ b/spec/javascripts/stimulus/tag_list_input_controller_test.js
@@ -0,0 +1,126 @@
+/**
+ * @jest-environment jsdom
+ */
+
+import { Application } from "stimulus";
+import tag_list_input_controller from "../../../app/components/tag_list_input_component/tag_list_input_controller";
+
+describe("TagListInputController", () => {
+  beforeAll(() => {
+    const application = Application.start();
+    application.register("tag-list-input-component--tag-list-input", tag_list_input_controller);
+  });
+
+  beforeEach(() => {
+    document.body.innerHTML = `
+      <div data-controller="tag-list-input-component--tag-list-input">
+        <input 
+          value="tag 1,tag 2,tag 3" 
+          data-tag-list-input-component--tag-list-input-target="tagList" 
+          type="hidden" 
+          name="variant_tag_list" id="variant_tag_list"
+        >
+        <div class="tags-input">
+          <div class="tags">
+            <ul class="tag-list" data-tag-list-input-component--tag-list-input-target="list">
+              <template data-tag-list-input-component--tag-list-input-target="template">
+                <li class="tag-item">
+                  <div class="tag-template">
+                  <span></span>
+                  <a 
+                    class="remove-button" 
+                    data-action="click->tag-list-input-component--tag-list-input#removeTag"
+                  >✖</a>
+                  </div>
+                </li>
+              </template>
+              <li class="tag-item">
+                <div class="tag-template">
+                  <span>tag 1</span>
+                  <a 
+                    class="remove-button" 
+                    data-action="click->tag-list-input-component--tag-list-input#removeTag"
+                  >✖</a>
+                </div>
+              </li>
+              <li class="tag-item">
+                <div class="tag-template">
+                  <span>tag 2</span>
+                  <a 
+                    class="remove-button" 
+                    data-action="click->tag-list-input-component--tag-list-input#removeTag"
+                  >✖</a>
+                </div>
+              </li>
+              <li class="tag-item">
+                <div class="tag-template">
+                  <span>tag 3</span>
+                  <a 
+                    class="remove-button" 
+                    data-action="click->tag-list-input-component--tag-list-input#removeTag"
+                  >✖</a>
+                </div>
+              </li>
+            </ul>
+            <input 
+              type="text" 
+              name="variant_add_tag" 
+              id="variant_add_tag" 
+              placeholder="Add a tag" 
+              data-action="keydown.enter->tag-list-input-component--tag-list-input#addTag keyup->tag-list-input-component--tag-list-input#filterInput" data-tag-list-input-component--tag-list-input-target="newTag"
+              >
+          </div>
+        </div>
+      </div>`;
+  });
+
+  describe("addTag", () => {
+    beforeEach(() => {
+      variant_add_tag.value = "new_tag";
+      // { key: "Enter" }
+      variant_add_tag.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter" }));
+    });
+
+    it("updates the hidden input tag list", () => {
+      expect(variant_tag_list.value).toBe("tag 1,tag 2,tag 3,new_tag");
+    });
+
+    it("adds the new tag to the HTML tag list", () => {
+      const tagList = document.getElementsByClassName("tag-list")[0];
+
+      // 1 template + 3 tags + 1 new tag
+      expect(tagList.childElementCount).toBe(5);
+    });
+
+    it("clears the tag input", () => {
+      expect(variant_add_tag.value).toBe("");
+    });
+  });
+
+  describe("removeTag", () => {
+    beforeEach(() => {
+      const removeButtons = document.getElementsByClassName("remove-button");
+      // Click on tag 2
+      removeButtons[1].click();
+    });
+
+    it("updates the hidden input tag list", () => {
+      expect(variant_tag_list.value).toBe("tag 1,tag 3");
+    });
+
+    it("removes the tag from the HTML tag list", () => {
+      const tagList = document.getElementsByClassName("tag-list")[0];
+      // 1 template + 2 tags
+      expect(tagList.childElementCount).toBe(3);
+    });
+  });
+
+  describe("filterInput", () => {
+    it("removes comma from the tag input", () => {
+      variant_add_tag.value = "text"
+      variant_add_tag.dispatchEvent(new KeyboardEvent("keyup", { key: "," }));
+
+      expect(variant_add_tag.value).toBe("text");
+    });
+  });
+});

From 559249b6217b8de68fca53f0e2c8eefc9ff29c03 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 9 Apr 2025 13:37:16 +1000
Subject: [PATCH 09/24] Improve tagListInputController to integrate with
 bulkFormController

tagListInputController is not a standard form controller so we had to
make a few changes to integrate with the change tracking used int
bulkFormController. Mainly we make sure to manually trigger event when
deleting a tags, and we handle the change highlight.
---
 .../tag_list_input_component.html.haml        |  7 ++++---
 .../tag_list_input_component.scss             |  5 +++++
 .../tag_list_input_controller.js              | 19 +++++++++++++++++-
 .../controllers/bulk_form_controller.js       |  5 +++--
 .../tag_list_input_controller_test.js         | 20 ++++++++++++++++---
 5 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/app/components/tag_list_input_component/tag_list_input_component.html.haml b/app/components/tag_list_input_component/tag_list_input_component.html.haml
index 0149584b2c..e1e4440933 100644
--- a/app/components/tag_list_input_component/tag_list_input_component.html.haml
+++ b/app/components/tag_list_input_component/tag_list_input_component.html.haml
@@ -1,7 +1,8 @@
 - #%div{ "data-controller": "tag-list-input-component--tag-list-input" }
-%div{ "data-controller": "tag-list-input-component--tag-list-input" }
-  = f.hidden_field method.to_sym, value: tags.join(","), "data-tag-list-input-component--tag-list-input-target": "tagList"
-  .tags-input{  }
+%div{ "data-controller": "tag-list-input-component--tag-list-input", "data-tag-list-input-component--tag-list-input-highlight-class-value": "changed" }
+  - # We use display:none instead of hidden field, so changes to the value can be picked up by the bulkFormController 
+  = f.text_field method.to_sym, value: tags.join(","), "data-tag-list-input-component--tag-list-input-target": "tagList", "style": "display: none" 
+  .tags-input
     .tags
       %ul.tag-list{"data-tag-list-input-component--tag-list-input-target": "list"}
         %template{"data-tag-list-input-component--tag-list-input-target": "template"}
diff --git a/app/components/tag_list_input_component/tag_list_input_component.scss b/app/components/tag_list_input_component/tag_list_input_component.scss
index 64e7998d29..551cbebb8c 100644
--- a/app/components/tag_list_input_component/tag_list_input_component.scss
+++ b/app/components/tag_list_input_component/tag_list_input_component.scss
@@ -12,6 +12,11 @@
     height: 100%;
     box-shadow: none;
 
+    &.changed {
+      border: 1px solid $color-txt-changed-brd;
+      border-radius: 4px;
+    }
+
     .tag-list {
       margin: 0;
       padding: 0;
diff --git a/app/components/tag_list_input_component/tag_list_input_controller.js b/app/components/tag_list_input_component/tag_list_input_controller.js
index 1316b85855..c65c0aa230 100644
--- a/app/components/tag_list_input_component/tag_list_input_controller.js
+++ b/app/components/tag_list_input_component/tag_list_input_controller.js
@@ -2,6 +2,7 @@ import { Controller } from "stimulus";
 
 export default class extends Controller {
   static targets = ["tagList", "newTag", "template", "list"];
+  static values = { highlightClass: String };
 
   addTag() {
     // add to tagList
@@ -13,6 +14,8 @@ export default class extends Controller {
     spanElement.innerText = this.newTagTarget.value;
     this.listTarget.appendChild(newTagElement);
 
+    this.#highlightList();
+
     // Clear new tag value
     this.newTagTarget.value = "";
   }
@@ -27,6 +30,10 @@ export default class extends Controller {
     tags.splice(index, 1);
     this.tagListTarget.value = tags.join(",");
 
+    // manualy dispatch an Input event so the change gets picked up by the bulk form controller
+    this.tagListTarget.dispatchEvent(new InputEvent("input"));
+    this.#highlightList();
+
     // Remove HTML element from the list
     event.srcElement.parentElement.parentElement.remove();
   }
@@ -34,7 +41,17 @@ export default class extends Controller {
   // Strip comma from tag name
   filterInput(event) {
     if (event.key === ",") {
-      event.srcElement.value = event.srcElement.value.replace(",","");
+      event.srcElement.value = event.srcElement.value.replace(",", "");
+    }
+  }
+
+  // private
+
+  #highlightList() {
+    if (this.highlightClassValue !== "") {
+      // div with the tags class
+      const tagsInputDiv = this.tagListTarget.nextElementSibling.firstElementChild;
+      tagsInputDiv.classList.add(this.highlightClassValue);
     }
   }
 }
diff --git a/app/webpacker/controllers/bulk_form_controller.js b/app/webpacker/controllers/bulk_form_controller.js
index 5cffebaee9..7b6a308008 100644
--- a/app/webpacker/controllers/bulk_form_controller.js
+++ b/app/webpacker/controllers/bulk_form_controller.js
@@ -24,7 +24,7 @@ export default class BulkFormController extends Controller {
 
   connect() {
     // disable form submit via enter key, so we can use enter key to create new product tags
-    hotkeys('enter', function (event, handler) {
+    hotkeys("enter", function (event, handler) {
       event.preventDefault();
     });
 
@@ -32,7 +32,6 @@ export default class BulkFormController extends Controller {
     this.form = this.element;
 
     // Start listening for any changes within the form
-    // TODO make sure tag inpug appreas here when deleted
     this.#registerElements(this.form.elements);
 
     this.toggleFormChanged();
@@ -174,6 +173,8 @@ export default class BulkFormController extends Controller {
 
       return !areBothBlank && selectedOption !== defaultSelected;
     } else {
+      // This doesn't work with hidden field
+      //   Workaround: use a text field with "display:none;"
       return element.defaultValue !== undefined && element.value != element.defaultValue;
     }
   }
diff --git a/spec/javascripts/stimulus/tag_list_input_controller_test.js b/spec/javascripts/stimulus/tag_list_input_controller_test.js
index b9c266de1d..b054c2fba2 100644
--- a/spec/javascripts/stimulus/tag_list_input_controller_test.js
+++ b/spec/javascripts/stimulus/tag_list_input_controller_test.js
@@ -13,7 +13,10 @@ describe("TagListInputController", () => {
 
   beforeEach(() => {
     document.body.innerHTML = `
-      <div data-controller="tag-list-input-component--tag-list-input">
+      <div 
+        data-controller="tag-list-input-component--tag-list-input" 
+        data-tag-list-input-component--tag-list-input-highlight-class-value="changed"
+      >
         <input 
           value="tag 1,tag 2,tag 3" 
           data-tag-list-input-component--tag-list-input-target="tagList" 
@@ -77,7 +80,6 @@ describe("TagListInputController", () => {
   describe("addTag", () => {
     beforeEach(() => {
       variant_add_tag.value = "new_tag";
-      // { key: "Enter" }
       variant_add_tag.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter" }));
     });
 
@@ -95,6 +97,12 @@ describe("TagListInputController", () => {
     it("clears the tag input", () => {
       expect(variant_add_tag.value).toBe("");
     });
+
+    it("higlights the tag list", () => {
+      const tagList = document.getElementsByClassName("tags")[0];
+
+      expect(tagList.classList).toContain("changed");
+    });
   });
 
   describe("removeTag", () => {
@@ -113,11 +121,17 @@ describe("TagListInputController", () => {
       // 1 template + 2 tags
       expect(tagList.childElementCount).toBe(3);
     });
+
+    it("higlights the tag list", () => {
+      const tagList = document.getElementsByClassName("tags")[0];
+
+      expect(tagList.classList).toContain("changed");
+    });
   });
 
   describe("filterInput", () => {
     it("removes comma from the tag input", () => {
-      variant_add_tag.value = "text"
+      variant_add_tag.value = "text";
       variant_add_tag.dispatchEvent(new KeyboardEvent("keyup", { key: "," }));
 
       expect(variant_add_tag.value).toBe("text");

From 94cc774f27215844f40c79571dc9f6d8fecc5208 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 9 Apr 2025 14:56:13 +1000
Subject: [PATCH 10/24] Fix html used in test

It get rids of errors, even if they didn't affect the test result it's
noise we don't need
---
 .../stimulus/bulk_form_controller_test.js     | 98 ++++++++++---------
 1 file changed, 54 insertions(+), 44 deletions(-)

diff --git a/spec/javascripts/stimulus/bulk_form_controller_test.js b/spec/javascripts/stimulus/bulk_form_controller_test.js
index 2f1e8ea429..b99e24d581 100644
--- a/spec/javascripts/stimulus/bulk_form_controller_test.js
+++ b/spec/javascripts/stimulus/bulk_form_controller_test.js
@@ -33,19 +33,21 @@ describe("BulkFormController", () => {
         <form data-controller="bulk-form" data-bulk-form-disable-selector-value="#disable1,#disable2">
           <div id="actions" data-bulk-form-target="actions" class="hidden"></div>
           <div id="changed_summary" data-bulk-form-target="changedSummary" data-translation-key="changed_summary"></div>
-          <div data-record-id="1">
-            <input id="input1a" type="text" value="initial1a">
-            <input id="input1b" type="text" value="initial1b">
-            <select id="select1">
-              <option>one</option>
-              <option selected>two</option>
-            </select>
-            <button>a button is counted as a form element, but value is undefined</button>
-          </div>
-          <div data-record-id="2">
-            <input id="input2" type="text" value="initial2">
-          </div>
-          <input type="submit">
+          <table class="products">
+            <div data-record-id="1">
+              <input id="input1a" type="text" value="initial1a">
+              <input id="input1b" type="text" value="initial1b">
+              <select id="select1">
+                <option>one</option>
+                <option selected>two</option>
+              </select>
+              <button>a button is counted as a form element, but value is undefined</button>
+            </div>
+            <div data-record-id="2">
+              <input id="input2" type="text" value="initial2">
+            </div>
+            <input type="submit">
+          </table>
         </form>
       `;
     });
@@ -104,13 +106,15 @@ describe("BulkFormController", () => {
         beforeEach(() => {
           document.body.innerHTML = `
             <form data-controller="bulk-form" data-bulk-form-disable-selector-value="#disable1,#disable2">
-              <div data-record-id="1">
-                <select id="select1">
-                  <option value="1">one</option>
-                  <option value="2">two</option>
-                </select>
-              </div>
-              <input type="submit">
+              <table class="products">
+                <div data-record-id="1">
+                  <select id="select1">
+                    <option value="1">one</option>
+                    <option value="2">two</option>
+                  </select>
+                </div>
+                <input type="submit">
+              </table>
             </form>
           `;
         });
@@ -131,13 +135,15 @@ describe("BulkFormController", () => {
         beforeEach(() => {
           document.body.innerHTML = `
             <form data-controller="bulk-form" data-bulk-form-disable-selector-value="#disable1,#disable2">
-              <div data-record-id="1">
-                <select id="select1">
-                  <option value="">blank</option>
-                  <option value="1">one</option>
-                </select>
-              </div>
-              <input type="submit">
+              <table class="products">
+                <div data-record-id="1">
+                  <select id="select1">
+                    <option value="">blank</option>
+                    <option value="1">one</option>
+                  </select>
+                </div>
+                <input type="submit">
+              </table>
             </form>
           `;
         });
@@ -221,13 +227,15 @@ describe("BulkFormController", () => {
     beforeEach(() => {
       document.body.innerHTML = `
         <form data-controller="bulk-form" data-bulk-form-error-value="true">
-          <div id="actions" data-bulk-form-target="actions">
-            An error occurred.
-            <input type="submit">
-          </div>
-          <div data-record-id="1">
-            <input id="input1a" type="text" value="initial1a">
-          </div>
+          <table class="products">
+            <div id="actions" data-bulk-form-target="actions">
+              An error occurred.
+              <input type="submit">
+            </div>
+            <div data-record-id="1">
+              <input id="input1a" type="text" value="initial1a">
+            </div>
+          </table>
         </form>
       `;
 
@@ -258,16 +266,18 @@ describe("BulkFormController", () => {
     beforeEach(() => {
       document.body.innerHTML = `
         <form id="form" data-controller="bulk-form" data-action="custom-event->bulk-form#registerElements",
-          <div data-record-id="1">
-            <input id="input1a" type="text" value="initial1a">
-            <template id="template">
-              <input id="input1b" type="text" value="initial1b">
-            </template>
-          </div>
-          <div data-record-id="2">
-            <input id="input2" type="text" value="initial2">
-          </div>
-          <input type="submit">
+          <table class="products">
+            <div data-record-id="1">
+              <input id="input1a" type="text" value="initial1a">
+              <template id="template">
+                <input id="input1b" type="text" value="initial1b">
+              </template>
+            </div>
+            <div data-record-id="2">
+              <input id="input2" type="text" value="initial2">
+            </div>
+            <input type="submit">
+          </table>
         </form>
       `;
     });

From 9e1a80c327de2d6c579d0fc32fdd8f4eeb38551f Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 9 Apr 2025 14:59:37 +1000
Subject: [PATCH 11/24] Mock hotkeys.js

---
 spec/javascripts/stimulus/bulk_form_controller_test.js | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/spec/javascripts/stimulus/bulk_form_controller_test.js b/spec/javascripts/stimulus/bulk_form_controller_test.js
index b99e24d581..850b380f5b 100644
--- a/spec/javascripts/stimulus/bulk_form_controller_test.js
+++ b/spec/javascripts/stimulus/bulk_form_controller_test.js
@@ -9,6 +9,14 @@ describe("BulkFormController", () => {
   beforeAll(() => {
     const application = Application.start();
     application.register("bulk-form", bulk_form_controller);
+
+    // Mock hotkeys.js
+    const mockedHotkeys = jest.fn();
+    global.hotkeys = mockedHotkeys;
+  });
+
+  afterAll(() => {
+    delete global.hotkeys;
   });
 
   describe("Modifying input values", () => {

From 157de25f36d5adf9cc26a8495748af96e56bf434 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 9 Apr 2025 15:00:41 +1000
Subject: [PATCH 12/24] Prettyfy code

---
 .../stimulus/bulk_form_controller_test.js     | 122 +++++++++---------
 1 file changed, 61 insertions(+), 61 deletions(-)

diff --git a/spec/javascripts/stimulus/bulk_form_controller_test.js b/spec/javascripts/stimulus/bulk_form_controller_test.js
index 850b380f5b..987d690a70 100644
--- a/spec/javascripts/stimulus/bulk_form_controller_test.js
+++ b/spec/javascripts/stimulus/bulk_form_controller_test.js
@@ -23,16 +23,16 @@ describe("BulkFormController", () => {
     // Mock I18n. TODO: moved to a shared helper
     beforeAll(() => {
       const mockedT = jest.fn();
-      mockedT.mockImplementation((string, opts) => (string + ', ' + JSON.stringify(opts)));
+      mockedT.mockImplementation((string, opts) => string + ", " + JSON.stringify(opts));
 
-      global.I18n =  {
-        t: mockedT
+      global.I18n = {
+        t: mockedT,
       };
-    })
+    });
     // (jest still doesn't have aroundEach https://github.com/jestjs/jest/issues/4543 )
     afterAll(() => {
       delete global.I18n;
-    })
+    });
 
     beforeEach(() => {
       document.body.innerHTML = `
@@ -62,17 +62,17 @@ describe("BulkFormController", () => {
 
     describe("marking changed fields", () => {
       it("input: onInput", () => {
-        input1a.value = 'updated1a';
+        input1a.value = "updated1a";
         input1a.dispatchEvent(new Event("input"));
         // Expect only first field to show changed
-        expect(input1a.classList).toContain('changed');
-        expect(input1b.classList).not.toContain('changed');
-        expect(input2.classList).not.toContain('changed');
+        expect(input1a.classList).toContain("changed");
+        expect(input1b.classList).not.toContain("changed");
+        expect(input2.classList).not.toContain("changed");
 
         // Change back to original value
-        input1a.value = 'initial1a';
+        input1a.value = "initial1a";
         input1a.dispatchEvent(new Event("input"));
-        expect(input1a.classList).not.toContain('changed');
+        expect(input1a.classList).not.toContain("changed");
       });
 
       it("select: onInput", () => {
@@ -81,33 +81,33 @@ describe("BulkFormController", () => {
         select1.options[1].selected = false;
         select1.dispatchEvent(new Event("input"));
         // Expect select to show changed
-        expect(input1a.classList).not.toContain('changed');
-        expect(input1b.classList).not.toContain('changed');
-        expect(select1.classList).toContain('changed');
+        expect(input1a.classList).not.toContain("changed");
+        expect(input1b.classList).not.toContain("changed");
+        expect(select1.classList).toContain("changed");
 
         // Change back to original value
         select1.options[0].selected = false;
         select1.options[1].selected = true;
         select1.dispatchEvent(new Event("input"));
-        expect(select1.classList).not.toContain('changed');
+        expect(select1.classList).not.toContain("changed");
       });
 
       it("multiple fields", () => {
-        input1a.value = 'updated1a';
+        input1a.value = "updated1a";
         input1a.dispatchEvent(new Event("input"));
-        input2.value = 'updated2';
+        input2.value = "updated2";
         input2.dispatchEvent(new Event("input"));
         // Expect only first field to show changed
-        expect(input1a.classList).toContain('changed');
-        expect(input1b.classList).not.toContain('changed');
-        expect(input2.classList).toContain('changed');
+        expect(input1a.classList).toContain("changed");
+        expect(input1b.classList).not.toContain("changed");
+        expect(input2.classList).toContain("changed");
 
         // Change only one back to original value
-        input1a.value = 'initial1a';
+        input1a.value = "initial1a";
         input1a.dispatchEvent(new Event("input"));
-        expect(input1a.classList).not.toContain('changed');
-        expect(input1b.classList).not.toContain('changed');
-        expect(input2.classList).toContain('changed');
+        expect(input1a.classList).not.toContain("changed");
+        expect(input1b.classList).not.toContain("changed");
+        expect(input2.classList).toContain("changed");
       });
 
       describe("select not include_blank", () => {
@@ -129,13 +129,13 @@ describe("BulkFormController", () => {
 
         it("shows as changed", () => {
           // Expect select to show changed (select-one always has something selected)
-          expect(select1.classList).toContain('changed');
+          expect(select1.classList).toContain("changed");
 
           // Change selection
           select1.options[0].selected = false;
           select1.options[1].selected = true;
           select1.dispatchEvent(new Event("input"));
-          expect(select1.classList).toContain('changed');
+          expect(select1.classList).toContain("changed");
         });
       });
 
@@ -157,75 +157,75 @@ describe("BulkFormController", () => {
         });
 
         it("does not show as changed", () => {
-          expect(select1.classList).not.toContain('changed');
+          expect(select1.classList).not.toContain("changed");
 
           // Change selection
           select1.options[0].selected = false;
           select1.options[1].selected = true;
           select1.dispatchEvent(new Event("input"));
-          expect(select1.classList).toContain('changed');
+          expect(select1.classList).toContain("changed");
         });
       });
-    })
+    });
 
     describe("activating sections, and showing a summary", () => {
       // This scenario should probably be broken up into smaller units.
       it("counts changed records ", () => {
         // Record 1: First field changed
-        input1a.value = 'updated1a';
+        input1a.value = "updated1a";
         input1a.dispatchEvent(new Event("input"));
         // Actions and changed summary are shown, with other sections disabled
-        expect(actions.classList).not.toContain('hidden');
+        expect(actions.classList).not.toContain("hidden");
         expect(changed_summary.textContent).toBe('changed_summary, {"count":1}');
-        expect(disable1.classList).toContain('disabled-section');
+        expect(disable1.classList).toContain("disabled-section");
         expect(disable1_element.disabled).toBe(true);
-        expect(disable2.classList).toContain('disabled-section');
+        expect(disable2.classList).toContain("disabled-section");
         expect(disable2_element.disabled).toBe(true);
 
         // Record 1: Second field changed
-        input1b.value = 'updated1b';
+        input1b.value = "updated1b";
         input1b.dispatchEvent(new Event("input"));
         // Expect to show same summary translation
-        expect(actions.classList).not.toContain('hidden');
+        expect(actions.classList).not.toContain("hidden");
         expect(changed_summary.textContent).toBe('changed_summary, {"count":1}');
 
         // Record 2: has been changed
-        input2.value = 'updated2';
+        input2.value = "updated2";
         input2.dispatchEvent(new Event("input"));
         // Expect summary to count both records
-        expect(actions.classList).not.toContain('hidden');
+        expect(actions.classList).not.toContain("hidden");
         expect(changed_summary.textContent).toBe('changed_summary, {"count":2}');
 
         // Record 1: Change first field back to original value
-        input1a.value = 'initial1a';
+        input1a.value = "initial1a";
         input1a.dispatchEvent(new Event("input"));
         // Both records are still changed.
-        expect(input1a.classList).not.toContain('changed');
-        expect(input1b.classList).toContain('changed');
-        expect(input2.classList).toContain('changed');
-        expect(actions.classList).not.toContain('hidden');
+        expect(input1a.classList).not.toContain("changed");
+        expect(input1b.classList).toContain("changed");
+        expect(input2.classList).toContain("changed");
+        expect(actions.classList).not.toContain("hidden");
         expect(changed_summary.textContent).toBe('changed_summary, {"count":2}');
 
         // Record 1: Change second field back to original value
-        input1b.value = 'initial1b';
+        input1b.value = "initial1b";
         input1b.dispatchEvent(new Event("input"));
         // Both fields for record 1 show unchanged, but second record is still changed
-        expect(actions.classList).not.toContain('hidden');
+        expect(actions.classList).not.toContain("hidden");
         expect(changed_summary.textContent).toBe('changed_summary, {"count":1}');
-        expect(disable1.classList).toContain('disabled-section');
+        expect(disable1.classList).toContain("disabled-section");
         expect(disable1_element.disabled).toBe(true);
-        expect(disable2.classList).toContain('disabled-section');
+        expect(disable2.classList).toContain("disabled-section");
         expect(disable2_element.disabled).toBe(true);
 
         // Record 2: Change back to original value
-        input2.value = 'initial2';
+        input2.value = "initial2";
         input2.dispatchEvent(new Event("input"));
         // Actions are hidden and other sections are now re-enabled
-        expect(actions.classList).toContain('hidden');
+        expect(actions.classList).toContain("hidden");
         expect(changed_summary.textContent).toBe('changed_summary, {"count":0}');
-        expect(disable1.classList).not.toContain('disabled-section');
+        expect(disable1.classList).not.toContain("disabled-section");
         expect(disable1_element.disabled).toBe(false);
-        expect(disable2.classList).not.toContain('disabled-section');
+        expect(disable2.classList).not.toContain("disabled-section");
         expect(disable2_element.disabled).toBe(false);
       });
     });
@@ -254,19 +254,19 @@ describe("BulkFormController", () => {
 
     it("form actions section remains visible", () => {
       // Expect actions to remain visible
-      expect(actions.classList).not.toContain('hidden');
+      expect(actions.classList).not.toContain("hidden");
 
       // Record 1: First field changed
-      input1a.value = 'updated1a';
+      input1a.value = "updated1a";
       input1a.dispatchEvent(new Event("input"));
       // Expect actions to remain visible
-      expect(actions.classList).not.toContain('hidden');
+      expect(actions.classList).not.toContain("hidden");
 
       // Change back to original value
-      input1a.value = 'initial1a';
+      input1a.value = "initial1a";
       input1a.dispatchEvent(new Event("input"));
       // Expect actions to remain visible
-      expect(actions.classList).not.toContain('hidden');
+      expect(actions.classList).not.toContain("hidden");
     });
   });
 
@@ -300,18 +300,18 @@ describe("BulkFormController", () => {
       });
 
       it("onInput", () => {
-        input1b.value = 'updated1b';
+        input1b.value = "updated1b";
         input1b.dispatchEvent(new Event("input"));
         // Expect only updated field to show changed
-        expect(input1b.classList).toContain('changed');
-        expect(input2.classList).not.toContain('changed');
+        expect(input1b.classList).toContain("changed");
+        expect(input2.classList).not.toContain("changed");
 
         // Change back to original value
-        input1b.value = 'initial1b';
+        input1b.value = "initial1b";
         input1b.dispatchEvent(new Event("input"));
-        expect(input1b.classList).not.toContain('changed');
+        expect(input1b.classList).not.toContain("changed");
       });
-    })
+    });
   });
 
   // unable to test disconnect at this stage

From 6417c87047e464f408063ee400d436b88b5bd894 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Thu, 10 Apr 2025 10:14:44 +1000
Subject: [PATCH 13/24] Add visual feedback when adding tag errors

Highlight the tag name in red, if trying to add a tag that already
exists.
---
 .../tag_list_input_component.scss             |  4 +++
 .../tag_list_input_controller.js              | 21 ++++++++++++--
 .../tag_list_input_controller_test.js         | 29 +++++++++++++++++++
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/app/components/tag_list_input_component/tag_list_input_component.scss b/app/components/tag_list_input_component/tag_list_input_component.scss
index 551cbebb8c..27a347b274 100644
--- a/app/components/tag_list_input_component/tag_list_input_component.scss
+++ b/app/components/tag_list_input_component/tag_list_input_component.scss
@@ -17,6 +17,10 @@
       border-radius: 4px;
     }
 
+    .tag-error {
+      color: $color-error;
+    }
+
     .tag-list {
       margin: 0;
       padding: 0;
diff --git a/app/components/tag_list_input_component/tag_list_input_controller.js b/app/components/tag_list_input_component/tag_list_input_controller.js
index c65c0aa230..0c5d3c8a19 100644
--- a/app/components/tag_list_input_component/tag_list_input_controller.js
+++ b/app/components/tag_list_input_component/tag_list_input_controller.js
@@ -5,13 +5,23 @@ export default class extends Controller {
   static values = { highlightClass: String };
 
   addTag() {
+    // Check if tag already exist
+    const newTagName = this.newTagTarget.value
+    const tags = this.tagListTarget.value.split(",");
+    const index = tags.indexOf(newTagName);
+    if (index != -1) {
+      // highlight the value in red
+      this.newTagTarget.classList.add("tag-error") 
+      return
+    } 
+    
     // add to tagList
-    this.tagListTarget.value = this.tagListTarget.value.concat(`,${this.newTagTarget.value}`);
+    this.tagListTarget.value = this.tagListTarget.value.concat(`,${newTagName}`);
 
     // Create new li component with value
     const newTagElement = this.templateTarget.content.cloneNode(true);
     const spanElement = newTagElement.querySelector("span");
-    spanElement.innerText = this.newTagTarget.value;
+    spanElement.innerText = newTagName;
     this.listTarget.appendChild(newTagElement);
 
     this.#highlightList();
@@ -38,8 +48,13 @@ export default class extends Controller {
     event.srcElement.parentElement.parentElement.remove();
   }
 
-  // Strip comma from tag name
   filterInput(event) {
+    // clear error class if key is not enter
+    if (event.key !== "Enter") {
+      this.newTagTarget.classList.remove("tag-error") 
+    }
+    
+    // Strip comma from tag name
     if (event.key === ",") {
       event.srcElement.value = event.srcElement.value.replace(",", "");
     }
diff --git a/spec/javascripts/stimulus/tag_list_input_controller_test.js b/spec/javascripts/stimulus/tag_list_input_controller_test.js
index b054c2fba2..fa535cc9f4 100644
--- a/spec/javascripts/stimulus/tag_list_input_controller_test.js
+++ b/spec/javascripts/stimulus/tag_list_input_controller_test.js
@@ -103,6 +103,26 @@ describe("TagListInputController", () => {
 
       expect(tagList.classList).toContain("changed");
     });
+
+    describe("when tag already exist", () => {
+      beforeEach(() => {
+        // Trying to add an existing tag
+        variant_add_tag.value = "tag 2";
+        variant_add_tag.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter" }));
+      });
+
+      it("doesn't add the tag", () => {
+        const tagList = document.getElementsByClassName("tag-list")[0];
+
+        // 1 template + 4 tags
+        expect(tagList.childElementCount).toBe(5);
+        expect(variant_add_tag.value).toBe("tag 2");
+      });
+
+      it("highlights the new tag name in red", () => {
+        expect(variant_add_tag.classList).toContain("tag-error")
+      });
+    })
   });
 
   describe("removeTag", () => {
@@ -136,5 +156,14 @@ describe("TagListInputController", () => {
 
       expect(variant_add_tag.value).toBe("text");
     });
+
+    it("removes error highlight", () => {
+      variant_add_tag.value = "text";
+      variant_add_tag.classList.add("tag-error")
+
+      variant_add_tag.dispatchEvent(new KeyboardEvent("keyup", { key: "a" }));
+
+      expect(variant_add_tag.classList).not.toContain("tag-error")
+    });
   });
 });

From 90ca22468034c147adb7e6192ec2c149cd424faa Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Mon, 14 Apr 2025 12:04:25 +1000
Subject: [PATCH 14/24] Fix tag display for long name

---
 .../tag_list_input_component/tag_list_input_component.scss       | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/components/tag_list_input_component/tag_list_input_component.scss b/app/components/tag_list_input_component/tag_list_input_component.scss
index 27a347b274..3589f76101 100644
--- a/app/components/tag_list_input_component/tag_list_input_component.scss
+++ b/app/components/tag_list_input_component/tag_list_input_component.scss
@@ -42,7 +42,6 @@
         Helvetica,
         Arial,
         sans-serif;
-      height: 25px;
       line-height: 25px;
       border: none;
       box-shadow: none;

From aba624073680ee4fa4db4098bc991d1bb132b7f6 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 15 Apr 2025 13:14:10 +1000
Subject: [PATCH 15/24] Per review, use css :has() pseudo class

It saves writing some custom javascript, less code to maintain!
---
 .../tag_list_input_component.html.haml          |  7 +++----
 .../tag_list_input_component.scss               |  2 +-
 .../tag_list_input_controller.js                | 14 --------------
 .../stimulus/tag_list_input_controller_test.js  | 17 +----------------
 4 files changed, 5 insertions(+), 35 deletions(-)

diff --git a/app/components/tag_list_input_component/tag_list_input_component.html.haml b/app/components/tag_list_input_component/tag_list_input_component.html.haml
index e1e4440933..52b024c25a 100644
--- a/app/components/tag_list_input_component/tag_list_input_component.html.haml
+++ b/app/components/tag_list_input_component/tag_list_input_component.html.haml
@@ -1,9 +1,8 @@
-- #%div{ "data-controller": "tag-list-input-component--tag-list-input" }
-%div{ "data-controller": "tag-list-input-component--tag-list-input", "data-tag-list-input-component--tag-list-input-highlight-class-value": "changed" }
-  - # We use display:none instead of hidden field, so changes to the value can be picked up by the bulkFormController 
-  = f.text_field method.to_sym, value: tags.join(","), "data-tag-list-input-component--tag-list-input-target": "tagList", "style": "display: none" 
+%div{ "data-controller": "tag-list-input-component--tag-list-input" }
   .tags-input
     .tags
+      - # We use display:none instead of hidden field, so changes to the value can be picked up by the bulkFormController 
+      = f.text_field method.to_sym, value: tags.join(","), "data-tag-list-input-component--tag-list-input-target": "tagList", "style": "display: none" 
       %ul.tag-list{"data-tag-list-input-component--tag-list-input-target": "list"}
         %template{"data-tag-list-input-component--tag-list-input-target": "template"}
           %li.tag-item
diff --git a/app/components/tag_list_input_component/tag_list_input_component.scss b/app/components/tag_list_input_component/tag_list_input_component.scss
index 3589f76101..67982bb022 100644
--- a/app/components/tag_list_input_component/tag_list_input_component.scss
+++ b/app/components/tag_list_input_component/tag_list_input_component.scss
@@ -12,7 +12,7 @@
     height: 100%;
     box-shadow: none;
 
-    &.changed {
+    &:has(.changed) {
       border: 1px solid $color-txt-changed-brd;
       border-radius: 4px;
     }
diff --git a/app/components/tag_list_input_component/tag_list_input_controller.js b/app/components/tag_list_input_component/tag_list_input_controller.js
index 0c5d3c8a19..1cfa219229 100644
--- a/app/components/tag_list_input_component/tag_list_input_controller.js
+++ b/app/components/tag_list_input_component/tag_list_input_controller.js
@@ -2,7 +2,6 @@ import { Controller } from "stimulus";
 
 export default class extends Controller {
   static targets = ["tagList", "newTag", "template", "list"];
-  static values = { highlightClass: String };
 
   addTag() {
     // Check if tag already exist
@@ -24,8 +23,6 @@ export default class extends Controller {
     spanElement.innerText = newTagName;
     this.listTarget.appendChild(newTagElement);
 
-    this.#highlightList();
-
     // Clear new tag value
     this.newTagTarget.value = "";
   }
@@ -42,7 +39,6 @@ export default class extends Controller {
 
     // manualy dispatch an Input event so the change gets picked up by the bulk form controller
     this.tagListTarget.dispatchEvent(new InputEvent("input"));
-    this.#highlightList();
 
     // Remove HTML element from the list
     event.srcElement.parentElement.parentElement.remove();
@@ -59,14 +55,4 @@ export default class extends Controller {
       event.srcElement.value = event.srcElement.value.replace(",", "");
     }
   }
-
-  // private
-
-  #highlightList() {
-    if (this.highlightClassValue !== "") {
-      // div with the tags class
-      const tagsInputDiv = this.tagListTarget.nextElementSibling.firstElementChild;
-      tagsInputDiv.classList.add(this.highlightClassValue);
-    }
-  }
 }
diff --git a/spec/javascripts/stimulus/tag_list_input_controller_test.js b/spec/javascripts/stimulus/tag_list_input_controller_test.js
index fa535cc9f4..99205bf068 100644
--- a/spec/javascripts/stimulus/tag_list_input_controller_test.js
+++ b/spec/javascripts/stimulus/tag_list_input_controller_test.js
@@ -13,10 +13,7 @@ describe("TagListInputController", () => {
 
   beforeEach(() => {
     document.body.innerHTML = `
-      <div 
-        data-controller="tag-list-input-component--tag-list-input" 
-        data-tag-list-input-component--tag-list-input-highlight-class-value="changed"
-      >
+      <div data-controller="tag-list-input-component--tag-list-input">
         <input 
           value="tag 1,tag 2,tag 3" 
           data-tag-list-input-component--tag-list-input-target="tagList" 
@@ -98,12 +95,6 @@ describe("TagListInputController", () => {
       expect(variant_add_tag.value).toBe("");
     });
 
-    it("higlights the tag list", () => {
-      const tagList = document.getElementsByClassName("tags")[0];
-
-      expect(tagList.classList).toContain("changed");
-    });
-
     describe("when tag already exist", () => {
       beforeEach(() => {
         // Trying to add an existing tag
@@ -141,12 +132,6 @@ describe("TagListInputController", () => {
       // 1 template + 2 tags
       expect(tagList.childElementCount).toBe(3);
     });
-
-    it("higlights the tag list", () => {
-      const tagList = document.getElementsByClassName("tags")[0];
-
-      expect(tagList.classList).toContain("changed");
-    });
   });
 
   describe("filterInput", () => {

From 034feabcff615cb89669a94afccd7482e0cb6fba Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 15 Apr 2025 13:15:49 +1000
Subject: [PATCH 16/24] Javascript code linting

---
 .../tag_list_input_controller.js                   | 14 +++++++-------
 .../stimulus/tag_list_input_controller_test.js     |  8 ++++----
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/app/components/tag_list_input_component/tag_list_input_controller.js b/app/components/tag_list_input_component/tag_list_input_controller.js
index 1cfa219229..b24212fd13 100644
--- a/app/components/tag_list_input_component/tag_list_input_controller.js
+++ b/app/components/tag_list_input_component/tag_list_input_controller.js
@@ -5,15 +5,15 @@ export default class extends Controller {
 
   addTag() {
     // Check if tag already exist
-    const newTagName = this.newTagTarget.value
+    const newTagName = this.newTagTarget.value;
     const tags = this.tagListTarget.value.split(",");
     const index = tags.indexOf(newTagName);
     if (index != -1) {
       // highlight the value in red
-      this.newTagTarget.classList.add("tag-error") 
-      return
-    } 
-    
+      this.newTagTarget.classList.add("tag-error");
+      return;
+    }
+
     // add to tagList
     this.tagListTarget.value = this.tagListTarget.value.concat(`,${newTagName}`);
 
@@ -47,9 +47,9 @@ export default class extends Controller {
   filterInput(event) {
     // clear error class if key is not enter
     if (event.key !== "Enter") {
-      this.newTagTarget.classList.remove("tag-error") 
+      this.newTagTarget.classList.remove("tag-error");
     }
-    
+
     // Strip comma from tag name
     if (event.key === ",") {
       event.srcElement.value = event.srcElement.value.replace(",", "");
diff --git a/spec/javascripts/stimulus/tag_list_input_controller_test.js b/spec/javascripts/stimulus/tag_list_input_controller_test.js
index 99205bf068..0efef0b051 100644
--- a/spec/javascripts/stimulus/tag_list_input_controller_test.js
+++ b/spec/javascripts/stimulus/tag_list_input_controller_test.js
@@ -111,9 +111,9 @@ describe("TagListInputController", () => {
       });
 
       it("highlights the new tag name in red", () => {
-        expect(variant_add_tag.classList).toContain("tag-error")
+        expect(variant_add_tag.classList).toContain("tag-error");
       });
-    })
+    });
   });
 
   describe("removeTag", () => {
@@ -144,11 +144,11 @@ describe("TagListInputController", () => {
 
     it("removes error highlight", () => {
       variant_add_tag.value = "text";
-      variant_add_tag.classList.add("tag-error")
+      variant_add_tag.classList.add("tag-error");
 
       variant_add_tag.dispatchEvent(new KeyboardEvent("keyup", { key: "a" }));
 
-      expect(variant_add_tag.classList).not.toContain("tag-error")
+      expect(variant_add_tag.classList).not.toContain("tag-error");
     });
   });
 });

From 6ff47eab3b87ff8b934e66fe15974de9bfb919bd Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 15 Apr 2025 14:59:47 +1000
Subject: [PATCH 17/24] Clean up css, we dont' need a different font for tags

---
 .../tag_list_input_component.scss                | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/app/components/tag_list_input_component/tag_list_input_component.scss b/app/components/tag_list_input_component/tag_list_input_component.scss
index 67982bb022..7e3882214c 100644
--- a/app/components/tag_list_input_component/tag_list_input_component.scss
+++ b/app/components/tag_list_input_component/tag_list_input_component.scss
@@ -28,20 +28,12 @@
     }
 
     li.tag-item {
-      -webkit-border-radius: 3px;
-      -moz-border-radius: 3px;
-      -ms-border-radius: 3px;
-      -o-border-radius: 3px;
       border-radius: 3px;
       margin: 2px 0 2px 3px;
       padding: 0 5px;
       display: inline-block;
       float: left;
-      font:
-        14px "Helvetica Neue",
-        Helvetica,
-        Arial,
-        sans-serif;
+      font-size: 14px;
       line-height: 25px;
       border: none;
       box-shadow: none;
@@ -70,11 +62,7 @@
       padding: 0 0 0 5px;
       float: left;
       height: 26px;
-      font:
-        14px "Helvetica Neue",
-        Helvetica,
-        Arial,
-        sans-serif;
+      font-size: 14px;
     }
   }
 }

From 31afdfd8c453a1e208febc781bce7656ae5f111f Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 15 Apr 2025 15:10:32 +1000
Subject: [PATCH 18/24] Disable "enter" hotkeys only on tag input element

We are still able to use enter to submit the form on anyother input.The
tag input creates a new tag when enter is pressed
---
 .../tag_list_input_component/tag_list_input_controller.js | 8 ++++++++
 app/webpacker/controllers/bulk_form_controller.js         | 5 -----
 spec/javascripts/stimulus/bulk_form_controller_test.js    | 8 --------
 .../stimulus/tag_list_input_controller_test.js            | 8 ++++++++
 4 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/app/components/tag_list_input_component/tag_list_input_controller.js b/app/components/tag_list_input_component/tag_list_input_controller.js
index b24212fd13..3d0ca779aa 100644
--- a/app/components/tag_list_input_component/tag_list_input_controller.js
+++ b/app/components/tag_list_input_component/tag_list_input_controller.js
@@ -3,6 +3,14 @@ import { Controller } from "stimulus";
 export default class extends Controller {
   static targets = ["tagList", "newTag", "template", "list"];
 
+  connect() {
+    // form hotkeys are enabled for "input" scope, we disable the form submit via enter
+    // on the tag input, so we can use the enter key to create new product tag
+    hotkeys("enter", { scope: "input", element: this.newTagTarget }, function () {
+      event.preventDefault();
+    });
+  }
+
   addTag() {
     // Check if tag already exist
     const newTagName = this.newTagTarget.value;
diff --git a/app/webpacker/controllers/bulk_form_controller.js b/app/webpacker/controllers/bulk_form_controller.js
index 7b6a308008..6e82d2ce33 100644
--- a/app/webpacker/controllers/bulk_form_controller.js
+++ b/app/webpacker/controllers/bulk_form_controller.js
@@ -23,11 +23,6 @@ export default class BulkFormController extends Controller {
   recordElements = {};
 
   connect() {
-    // disable form submit via enter key, so we can use enter key to create new product tags
-    hotkeys("enter", function (event, handler) {
-      event.preventDefault();
-    });
-
     this.submitting = false;
     this.form = this.element;
 
diff --git a/spec/javascripts/stimulus/bulk_form_controller_test.js b/spec/javascripts/stimulus/bulk_form_controller_test.js
index 987d690a70..b09d116b0f 100644
--- a/spec/javascripts/stimulus/bulk_form_controller_test.js
+++ b/spec/javascripts/stimulus/bulk_form_controller_test.js
@@ -9,14 +9,6 @@ describe("BulkFormController", () => {
   beforeAll(() => {
     const application = Application.start();
     application.register("bulk-form", bulk_form_controller);
-
-    // Mock hotkeys.js
-    const mockedHotkeys = jest.fn();
-    global.hotkeys = mockedHotkeys;
-  });
-
-  afterAll(() => {
-    delete global.hotkeys;
   });
 
   describe("Modifying input values", () => {
diff --git a/spec/javascripts/stimulus/tag_list_input_controller_test.js b/spec/javascripts/stimulus/tag_list_input_controller_test.js
index 0efef0b051..7ebfdadcb4 100644
--- a/spec/javascripts/stimulus/tag_list_input_controller_test.js
+++ b/spec/javascripts/stimulus/tag_list_input_controller_test.js
@@ -9,6 +9,14 @@ describe("TagListInputController", () => {
   beforeAll(() => {
     const application = Application.start();
     application.register("tag-list-input-component--tag-list-input", tag_list_input_controller);
+
+    // Mock hotkeys.js
+    const mockedHotkeys = jest.fn();
+    global.hotkeys = mockedHotkeys;
+  });
+
+  afterAll(() => {
+    delete global.hotkeys;
   });
 
   beforeEach(() => {

From 1479be787bc7691365698df024e475f0f348d859 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 15 Apr 2025 15:31:31 +1000
Subject: [PATCH 19/24] Prevent adding empty tag

---
 .../tag_list_input_controller.js                     |  6 +++++-
 .../stimulus/tag_list_input_controller_test.js       | 12 ++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/app/components/tag_list_input_component/tag_list_input_controller.js b/app/components/tag_list_input_component/tag_list_input_controller.js
index 3d0ca779aa..f0e9e3555b 100644
--- a/app/components/tag_list_input_component/tag_list_input_controller.js
+++ b/app/components/tag_list_input_component/tag_list_input_controller.js
@@ -13,7 +13,11 @@ export default class extends Controller {
 
   addTag() {
     // Check if tag already exist
-    const newTagName = this.newTagTarget.value;
+    const newTagName = this.newTagTarget.value.trim();
+    if (newTagName.length == 0) {
+      return;
+    }
+
     const tags = this.tagListTarget.value.split(",");
     const index = tags.indexOf(newTagName);
     if (index != -1) {
diff --git a/spec/javascripts/stimulus/tag_list_input_controller_test.js b/spec/javascripts/stimulus/tag_list_input_controller_test.js
index 7ebfdadcb4..a0c9f89eb8 100644
--- a/spec/javascripts/stimulus/tag_list_input_controller_test.js
+++ b/spec/javascripts/stimulus/tag_list_input_controller_test.js
@@ -103,6 +103,18 @@ describe("TagListInputController", () => {
       expect(variant_add_tag.value).toBe("");
     });
 
+    describe("with an empty new tag", () => {
+      it("doesn't add the tag", () => {
+        variant_add_tag.value = " ";
+        variant_add_tag.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter" }));
+
+        const tagList = document.getElementsByClassName("tag-list")[0];
+
+        // 1 template + 3 tags + new tag (added in the beforeEach)
+        expect(tagList.childElementCount).toBe(5);
+      });
+    });
+
     describe("when tag already exist", () => {
       beforeEach(() => {
         // Trying to add an existing tag

From b29345007d58c39ff0c90ce58a9a7b7176e79548 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Tue, 22 Apr 2025 13:57:55 +1000
Subject: [PATCH 20/24] Increase query count to take into account tag query

---
 spec/jobs/open_order_cycle_job_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/jobs/open_order_cycle_job_spec.rb b/spec/jobs/open_order_cycle_job_spec.rb
index c6d1d1d27e..bf7c84d5ab 100644
--- a/spec/jobs/open_order_cycle_job_spec.rb
+++ b/spec/jobs/open_order_cycle_job_spec.rb
@@ -71,7 +71,7 @@ RSpec.describe OpenOrderCycleJob do
         .and change { variant.on_demand }.to(true)
         .and change { variant.on_hand }.by(0)
         .and change { variant_discontinued.on_hand }.to(0)
-        .and query_database 58
+        .and query_database 59
     end
   end
 

From bb882ddfa35212e5bc240266e95e9daf71844207 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Thu, 24 Apr 2025 13:53:23 +1000
Subject: [PATCH 21/24] Per review, simplify disabling of default action

Turns out you can just call `event.preventDefault()` on the action and
there is no need actually use hotkeys to do that.
---
 .../tag_list_input_controller.js                      | 11 +++--------
 .../stimulus/tag_list_input_controller_test.js        |  8 --------
 2 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/app/components/tag_list_input_component/tag_list_input_controller.js b/app/components/tag_list_input_component/tag_list_input_controller.js
index f0e9e3555b..47ddf44e23 100644
--- a/app/components/tag_list_input_component/tag_list_input_controller.js
+++ b/app/components/tag_list_input_component/tag_list_input_controller.js
@@ -3,15 +3,10 @@ import { Controller } from "stimulus";
 export default class extends Controller {
   static targets = ["tagList", "newTag", "template", "list"];
 
-  connect() {
-    // form hotkeys are enabled for "input" scope, we disable the form submit via enter
-    // on the tag input, so we can use the enter key to create new product tag
-    hotkeys("enter", { scope: "input", element: this.newTagTarget }, function () {
-      event.preventDefault();
-    });
-  }
+  addTag(event) {
+    // prevent hotkey form submitting the form (default action for "enter" key)
+    event.preventDefault();
 
-  addTag() {
     // Check if tag already exist
     const newTagName = this.newTagTarget.value.trim();
     if (newTagName.length == 0) {
diff --git a/spec/javascripts/stimulus/tag_list_input_controller_test.js b/spec/javascripts/stimulus/tag_list_input_controller_test.js
index a0c9f89eb8..dd8461a360 100644
--- a/spec/javascripts/stimulus/tag_list_input_controller_test.js
+++ b/spec/javascripts/stimulus/tag_list_input_controller_test.js
@@ -9,14 +9,6 @@ describe("TagListInputController", () => {
   beforeAll(() => {
     const application = Application.start();
     application.register("tag-list-input-component--tag-list-input", tag_list_input_controller);
-
-    // Mock hotkeys.js
-    const mockedHotkeys = jest.fn();
-    global.hotkeys = mockedHotkeys;
-  });
-
-  afterAll(() => {
-    delete global.hotkeys;
   });
 
   beforeEach(() => {

From df785e907dc07bd07a8e0a55776281459fd09c42 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <40413322+rioug@users.noreply.github.com>
Date: Wed, 30 Apr 2025 13:53:30 +1000
Subject: [PATCH 22/24] Update
 app/components/tag_list_input_component/tag_list_input_controller.js

Co-authored-by: Maikel <maikel@email.org.au>
---
 .../tag_list_input_component/tag_list_input_controller.js     | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/app/components/tag_list_input_component/tag_list_input_controller.js b/app/components/tag_list_input_component/tag_list_input_controller.js
index 47ddf44e23..5fb0795e59 100644
--- a/app/components/tag_list_input_component/tag_list_input_controller.js
+++ b/app/components/tag_list_input_component/tag_list_input_controller.js
@@ -40,9 +40,7 @@ export default class extends Controller {
 
     // Remove tag from list
     const tags = this.tagListTarget.value.split(",");
-    const index = tags.indexOf(tagName);
-    tags.splice(index, 1);
-    this.tagListTarget.value = tags.join(",");
+    this.tagListTarget.value = tags.filter(tag => tag != tagName).join(",");
 
     // manualy dispatch an Input event so the change gets picked up by the bulk form controller
     this.tagListTarget.dispatchEvent(new InputEvent("input"));

From 212b122700df4eb99a7fb21032b774966467724e Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 30 Apr 2025 13:49:13 +1000
Subject: [PATCH 23/24] Add translation for default placeholder in
 TagListInputComponent

---
 app/components/tag_list_input_component.rb | 3 ++-
 config/locales/en.yml                      | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/app/components/tag_list_input_component.rb b/app/components/tag_list_input_component.rb
index 0897da332d..e83b266a47 100644
--- a/app/components/tag_list_input_component.rb
+++ b/app/components/tag_list_input_component.rb
@@ -2,7 +2,8 @@
 
 class TagListInputComponent < ViewComponent::Base
   # method in a "hidden_field" form helper and is the method used to get a list of tag on the model
-  def initialize(form:, method:, tags:, placeholder: "Add a tag")
+  def initialize(form:, method:, tags:,
+                 placeholder: I18n.t("components.tag_list_input.default_placeholder"))
     @f = form
     @method = method
     @tags = tags
diff --git a/config/locales/en.yml b/config/locales/en.yml
index adbae5c6c2..e6fd3f3e72 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -5057,6 +5057,8 @@ See the %{link} to find out more about %{sitename}'s features and to start using
     pagination:
       next: Next
       previous: Previous
+    tag_list_input:
+      default_placeholder: Add a tag 
 
 
   # Gem to prevent bot form submissions

From da8675826eea69e802e24912a4017159178780e2 Mon Sep 17 00:00:00 2001
From: Gaetan Craig-Riou <gaetan.riou@gmail.com>
Date: Wed, 30 Apr 2025 14:54:01 +1000
Subject: [PATCH 24/24] Per review, remove :variant_tag context

---
 app/models/spree/variant.rb                        | 2 +-
 app/services/permitted_attributes/variant.rb       | 2 +-
 app/views/admin/products_v3/_variant_row.html.haml | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb
index 20c60a9a5c..3f16cbcc70 100644
--- a/app/models/spree/variant.rb
+++ b/app/models/spree/variant.rb
@@ -18,7 +18,7 @@ module Spree
 
     acts_as_paranoid
 
-    acts_as_taggable_on :variant_tag
+    acts_as_taggable
 
     searchable_attributes :sku, :display_as, :display_name, :primary_taxon_id, :supplier_id
     searchable_associations :product, :default_price, :primary_taxon, :supplier
diff --git a/app/services/permitted_attributes/variant.rb b/app/services/permitted_attributes/variant.rb
index a538092699..d39e2fbde8 100644
--- a/app/services/permitted_attributes/variant.rb
+++ b/app/services/permitted_attributes/variant.rb
@@ -8,7 +8,7 @@ module PermittedAttributes
         :shipping_category_id, :price, :unit_value,
         :unit_description, :variant_unit, :variant_unit_name, :variant_unit_scale, :display_name,
         :display_as, :tax_category_id, :weight, :height, :width, :depth, :taxon_ids,
-        :primary_taxon_id, :supplier_id, :variant_tag_list
+        :primary_taxon_id, :supplier_id, :tag_list
       ]
     end
   end
diff --git a/app/views/admin/products_v3/_variant_row.html.haml b/app/views/admin/products_v3/_variant_row.html.haml
index 20ed7f8050..b454aa01c2 100644
--- a/app/views/admin/products_v3/_variant_row.html.haml
+++ b/app/views/admin/products_v3/_variant_row.html.haml
@@ -76,7 +76,7 @@
   = error_message_on variant, :tax_category
 - if feature?(:variant_tag, spree_current_user)
   %td.col-tags.field.naked_inputs
-    = render TagListInputComponent.new(form: f, method: "variant_tag_list", tags: variant.variant_tag_list, placeholder: t('.add_a_tag'))
+    = render TagListInputComponent.new(form: f, method: "tag_list", tags: variant.tag_list, placeholder: t('.add_a_tag'))
 %td.col-inherits_properties.align-left
   -# empty
 %td.align-right