Merge pull request #469 from platanus/fix-tags

feat(tags-input): rework to return array

Author: difernandez

CHANGELOG.md

@@ -7,6 +7,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 #### Breaking changes
 * Defines required ruby version to >=2.7.0 [#460](https://github.com/platanus/activeadmin_addons/pull/460)
 * Nested and search select now use the name of the association instead of the name of id [#462](https://github.com/platanus/activeadmin_addons/pull/462)
+* Tags input now returns an array of strings instead of a string [#469](https://github.com/platanus/activeadmin_addons/pull/469)
 
 #### Fixes
 * Include only items that belong to parent when using collection option in a nested input level [#463](https://github.com/platanus/activeadmin_addons/pull/463)

app/inputs/tags_input.rb

@@ -1,17 +1,12 @@
-class TagsInput < ActiveAdminAddons::InputBase
+class TagsInput < ActiveAdminAddons::SelectInputBase
   include ActiveAdminAddons::SelectHelpers
 
   def render_custom_input
-    if active_record_select?
-      return render_collection_tags
-    end
-
-    render_array_tags
+    render_collection_tags
   end
 
   def load_control_attributes
-    load_data_attr(:model, value: model_name)
-    load_data_attr(:method, value: method)
+    @options[:multiple] = true
     load_data_attr(:width)
 
     if active_record_select?
@@ -24,32 +19,8 @@ def load_control_attributes
 
   private
 
-  def render_array_tags
-    render_tags_control { build_hidden_control(prefixed_method, method_to_input_name, input_value) }
-  end
-
   def render_collection_tags
-    render_tags_control { render_selected_hidden_items }
-  end
-
-  def render_tags_control(&block)
     concat(label_html)
-    concat(block.call)
-    concat(builder.select(build_virtual_attr, [], {}, input_html_options))
-  end
-
-  def render_selected_hidden_items
-    template.content_tag(:div, id: selected_values_id) do
-      template.concat(build_hidden_control(empty_input_id, method_to_input_array_name, ""))
-      input_value.each do |item_id|
-        template.concat(
-          build_hidden_control(
-            method_to_input_id(item_id),
-            method_to_input_array_name,
-            item_id.to_s
-          )
-        )
-      end
-    end
+    concat(builder.select(method, [], input_options, input_html_options))
   end
 end

app/javascript/activeadmin_addons/inputs/slim-select-tags.js

@@ -2,9 +2,6 @@ const classes = ['tags-input'];
 
 // eslint-disable-next-line max-statements
 function settings(el) {
-  const model = el.dataset.model;
-  const method = el.dataset.method;
-  const prefix = `${model}_${method}`;
   const isRelation = el.dataset.relation === 'true';
   const collection = el.dataset.collection ? JSON.parse(el.dataset.collection) : null;
 
@@ -14,38 +11,7 @@ function settings(el) {
     return { ...rest, value: id, selected: !!item.selected };
   });
 
-  function fillHiddenInput(values) {
-    const hiddenInput = document.querySelector(`#${prefix}`);
-    hiddenInput.value = values.map(val => val.value).join();
-  }
-
-  const events = {
-    afterChange: (newVal) => {
-      if (isRelation) {
-        const selectedItemsContainer = document.querySelector(`#${prefix}_selected_values`);
-        const itemName = `${model}[${method}][]`;
-        selectedItemsContainer.innerHTML = '';
-
-        newVal.forEach((data) => {
-          const itemId = `${prefix}_${data.value}`;
-          if (document.querySelectorAll(`#${itemId}`).length > 0) {
-            return;
-          }
-
-          const hiddenInput = document.createElement('input');
-          hiddenInput.id = itemId;
-          hiddenInput.name = itemName;
-          hiddenInput.value = data.value;
-          hiddenInput.type = 'hidden';
-
-          selectedItemsContainer.appendChild(hiddenInput);
-        });
-      } else {
-        fillHiddenInput(newVal);
-      }
-    },
-  };
-
+  const events = {};
   if (!isRelation) {
     events.addable = (value) => value;
   }
@@ -56,12 +22,7 @@ function settings(el) {
   };
 }
 
-function init(el) {
-  el.multiple = true;
-}
-
 export {
   settings,
   classes,
-  init,
 };

docs/slim-select_tags.md

@@ -2,13 +2,13 @@
 
 ## Tagging
 
-To enable Slim Select with tags functionality you need to do the following:
+To allow selecting multiple values, you can use the `:tags` input type:
 
 ```ruby
 f.input :names, as: :tags
 ```
 
-You can load previous created tags using `collection` option passing an array of strings like this:
+You can load previously created tags using `collection` option passing an array of strings like this:
 
 ```ruby
 f.input :names, as: :tags, collection: ['Diego', 'Leandro', 'Guillermo']
@@ -36,9 +36,15 @@ So, in the ActiveAdmin's Event form, you can add:
 f.input :performer_ids, as: :tags, collection: Performer.all, display_name: :full_name
 ```
 
-> Remember: the input name must be: `performer_ids` not `performers` and you need to add to `permit_params` the `performer_ids: []` key.
+> Remember: the input name must be: `performer_ids` not `performers`.
 
-### Options
+## :warning: Gotchas
+
+Note that this input type uses a regular select with `multiple: true` under the hood. As such, it also returns an array of string, so **remember to add your attribute as an array to the permitted_params**, such as `performer_ids: []`.
+
+Moreover, this input is subject to [this same limitation](https://edgeapi.rubyonrails.org/classes/ActionView/Helpers/FormOptionsHelper.html#:~:text=Gotcha-,The%20HTML%20specification,-says%20when%20multiple) that multiple select inputs have: the returned array will always contain an empty string. When using with AR relations or something like [ActsAsTaggableOn](https://github.com/mbleigh/acts-as-taggable-on) this shouldn't be an issue, as an empty string won't trigger the creation of a new record. However, in other cases, like when using **postgres array column**, this might be a problem. Be sure to sanitize this value before saving in those cases.
+
+## Options
 
 * `display_name`: **(optional)** You can pass an optional `display_name` to set the attribute (or method) to show results on the select. It **defaults to**: `name`
 * `value`: **(optional)** You can pass an optional `value` to set the attribute (or method) to use when an item is selected. It **defaults to**: `id`

spec/features/inputs/tags_input_spec.rb

@@ -34,7 +34,7 @@
     end
   end
 
-  context "working with active record relations" do
+  context "when working with active record relations" do
     before do
       register_form(Invoice) do |f|
         f.input :item_ids, as: :tags, collection: Item.all
@@ -51,14 +51,12 @@
     context "with added item" do
       before { pick_slimselect_entered_option(@item1.name) }
 
-      it "adds/removes hidden item", js: true do
-        item_id = "#invoice_item_ids_#{@item1.id}"
-        input = find(item_id, visible: false)
-        expect(input.value).to eq(@item1.id.to_s)
-        expect(input[:name]).to eq("invoice[item_ids][]")
+      it "includes and then removes item from select value", js: true do
+        select_selector = "select[name='invoice[item_ids][]']"
+        expect(find(select_selector, visible: false).value).to include(@item1.id.to_s)
         find(".ss-value-delete").click
         sleep 0.5
-        expect { find(item_id, visible: false) }.to raise_error(Capybara::ElementNotFound)
+        expect(find(select_selector, visible: false).value).not_to include(@item1.id.to_s)
       end
 
       it "does not allow new items", js: true do
@@ -72,7 +70,7 @@
     end
   end
 
-  context "working with active record relations but alias" do
+  context "when working with active record relations but alias" do
     before do
       register_form(Invoice) do |f|
         f.input :other_item_ids, as: :tags, collection: Item.all
@@ -89,14 +87,12 @@
     context "with added item" do
       before { pick_slimselect_entered_option(@item1.name) }
 
-      it "adds/removes hidden item", js: true do
-        item_id = "#invoice_other_item_ids_#{@item1.id}"
-        input = find(item_id, visible: false)
-        expect(input.value).to eq(@item1.id.to_s)
-        expect(input[:name]).to eq("invoice[other_item_ids][]")
+      it "includes and then removes item from select value", js: true do
+        select_selector = "select[name='invoice[other_item_ids][]']"
+        expect(find(select_selector, visible: false).value).to include(@item1.id.to_s)
         find(".ss-value-delete").click
         sleep 0.5
-        expect { find(item_id, visible: false) }.to raise_error(Capybara::ElementNotFound)
+        expect(find(select_selector, visible: false).value).not_to include(@item1.id.to_s)
       end
     end
   end