Merge pull request openfoodfoundation#12194 from cyrillefr/Replace-dr…

…opdown_controller-with-generic-toggle-control_controller

Re-implement dropdown controller with html details element

app/views/spree/admin/orders/_bulk_actions.html.haml

@@ -3,23 +3,22 @@
     %span{ "data-controller": "checked-feedback", "data-checked-feedback-translation-value": "spree.admin.orders.index.selected" }
       = t("spree.admin.orders.index.selected", count: 0)
 
-  %div.plain.ofn-drop-down.disabled{ "data-checked-target": "disable", "data-controller": "dropdown", "data-action": "click->dropdown#toggle" }
-    %span{ class: 'icon-reorder' }
-      ="#{t('admin.actions')}".html_safe
-    %span
-      %i{ "data-dropdown-target": "arrow", "data-expanded-class": "icon-caret-up", "data-collapsed-class": "icon-caret-down" }
-
-    %div.menu{ "data-dropdown-target": "menu" }
-      %div.menu_item
-        %span.name{ "data-controller": "modal-link", "data-action": "click->modal-link#open", "data-modal-link-target-value": "resend_confirmation" }
-          = t('spree.admin.orders.index.resend_confirmation')
-      - if Spree::Config[:enable_invoices?]
+  %div.plain.ofn-drop-down.disabled{ "data-checked-target": "disable" }
+    %details{"data-controller": "dropdown"}
+      %summary
+        %span.icon-reorder
+          ="#{t('admin.actions')}".html_safe
+      %div.menu{"data-action": "click->dropdown#closeOnMenu"}
         %div.menu_item
-          %span.name{ "data-controller": "modal-link", "data-action": "click->modal-link#open", "data-modal-link-target-value": "send_invoice" }
-            = t('spree.admin.orders.index.send_invoice')
+          %span.name{ "data-controller": "modal-link", "data-action": "click->modal-link#open", "data-modal-link-target-value": "resend_confirmation" }
+            = t('spree.admin.orders.index.resend_confirmation')
+        - if Spree::Config[:enable_invoices?]
+          %div.menu_item
+            %span.name{ "data-controller": "modal-link", "data-action": "click->modal-link#open", "data-modal-link-target-value": "send_invoice" }
+              = t('spree.admin.orders.index.send_invoice')
+          %div.menu_item
+            %span.name{ "data-controller": "bulk-actions", "data-action": "click->bulk-actions#perform", "data-bulk-actions-reflex-value": "Admin::Orders#bulk_invoice" }
+              = t('spree.admin.orders.index.print_invoices')
         %div.menu_item
-          %span.name{ "data-controller": "bulk-actions", "data-action": "click->bulk-actions#perform", "data-bulk-actions-reflex-value": "Admin::Orders#bulk_invoice" }
-            = t('spree.admin.orders.index.print_invoices')
-      %div.menu_item
-        %span.name{ "data-controller": "modal-link", "data-action": "click->modal-link#open", "data-modal-link-target-value": "cancel_orders" }
-          = t('spree.admin.orders.index.cancel_orders')
+          %span.name{ "data-controller": "modal-link", "data-action": "click->modal-link#open", "data-modal-link-target-value": "cancel_orders" }
+            = t('spree.admin.orders.index.cancel_orders')

app/views/spree/admin/shared/_order_links.html.haml

@@ -1,20 +1,21 @@
 %li.links-dropdown#links-dropdown
-  .ofn-drop-down{"data-controller": "dropdown", "data-action": "click->dropdown#toggle" }
-    %span
-      %i.icon-check
-      = I18n.t 'admin.actions'
-      %i{ "data-dropdown-target": "arrow", "data-expanded-class": "icon-caret-up", "data-collapsed-class": "icon-caret-down" }
-    %div.menu.hidden{"data-dropdown-target": "menu"}
-      - order_links(@order).each do |link|
-        - if link[:name] == t(:ship_order)
-          %a.menu_item{ href: link[:url], target: link[:target] || "_self", data: { "modal-link-target-value": dom_id(@order, :ship), "action":  "click->modal-link#open", "controller": "modal-link" } }
-            %span
-              %i{ class: link[:icon] }
-            %span=link[:name]
-        - else
-          %a.menu_item{ href: link[:url], target: link[:target] || "_self", data: { method: link[:method], "ujs-navigate": link[:method] ? "false" : "undefined", confirm: link[:confirm] } }
-            %span
-              %i{ class: link[:icon] }
-            %span=link[:name]
+  .ofn-drop-down
+    %details{"data-controller": "dropdown"}
+      %summary
+        %span
+          %i.icon-check
+          = I18n.t 'admin.actions'
+      %div.menu{"data-action": "click->dropdown#closeOnMenu"}
+        - order_links(@order).each do |link|
+          - if link[:name] == t(:ship_order)
+            %a.menu_item{ href: link[:url], target: link[:target] || "_self", data: { "modal-link-target-value": dom_id(@order, :ship), "action":  "click->modal-link#open", "controller": "modal-link" } }
+              %span
+                %i{ class: link[:icon] }
+              %span=link[:name]
+          - else
+            %a.menu_item{ href: link[:url], target: link[:target] || "_self", data: { method: link[:method], "ujs-navigate": link[:method] ? "false" : "undefined", confirm: link[:confirm] } }
+              %span
+                %i{ class: link[:icon] }
+              %span=link[:name]
 
 = render 'spree/admin/shared/custom-confirm'

app/webpacker/controllers/checked_controller.js

@@ -41,13 +41,21 @@ export default class extends Controller {
     return this.countValue === this.checkboxTargets.length;
   }
 
+  #closeDetails(elmnt) {
+    if (elmnt.getElementsByTagName('details').length == 0)
+      return;
+
+   Array.from(elmnt.getElementsByTagName('details')).forEach((element) => element.open = false);
+ }
+
   #toggleDisabled() {
     if (!this.hasDisableTarget) {
       return;
     }
 
     if (this.#checkedCount() === 0) {
       this.disableTargets.forEach((element) => element.classList.add("disabled"));
+      this.disableTargets.forEach(this.#closeDetails);
     } else {
       this.disableTargets.forEach((element) => element.classList.remove("disabled"));
     }

app/webpacker/controllers/dropdown_controller.js

@@ -1,44 +1,29 @@
 import { Controller } from "stimulus";
 
 export default class extends Controller {
-  static targets = ["arrow", "menu"];
 
   connect() {
-    this.collapsedClasses = this.arrowTarget.dataset.collapsedClass.split(" ");
-    this.expandedClasses = this.arrowTarget.dataset.expandedClass.split(" ");
-    this.#hide();
-    document.addEventListener("click", this.#onBodyClick.bind(this));
+    document.body.addEventListener("click", this.#close.bind(this));
+    this.element.addEventListener("click", this.#stopPropagation.bind(this));
   }
 
   disconnect() {
-    document.removeEventListener("click", this.#onBodyClick);
+    document.removeEventListener("click", this.#close);
+    document.removeEventListener("click", this.#stopPropagation);
   }
 
-  toggle() {
-    if (this.element.classList.contains("disabled")) {
-      return;
-    }
-    if (this.menuTarget.classList.contains("hidden")) {
-      this.#show();
-    } else {
-      this.#hide();
-    }
+  closeOnMenu(event) {
+    this.#close();
+    this.#stopPropagation(event);
   }
 
-  #onBodyClick(event) {
-    if (!this.element.contains(event.target)) {
-      this.#hide();
-    }
-  }
+  // private
 
-  #show() {
-    this.menuTarget.classList.remove("hidden");
-    this.arrowTarget.classList.remove(...this.collapsedClasses);
-    this.arrowTarget.classList.add(...this.expandedClasses);
+  #close(event) {
+    this.element.open = false;
   }
-  #hide() {
-    this.menuTarget.classList.add("hidden");
-    this.arrowTarget.classList.remove(...this.expandedClasses);
-    this.arrowTarget.classList.add(...this.collapsedClasses);
+
+  #stopPropagation(event) {
+    event.stopPropagation();
   }
 }

app/webpacker/css/admin/dropdown.scss

@@ -47,6 +47,7 @@
 
   &.disabled {
     opacity: 0.5;
+    pointer-events: none;
 
     &:hover {
       cursor: default;
@@ -179,6 +180,34 @@
       background-color: #ededed;
     }
   }
+
+  > details {
+    // Override padding on ofn-drop-down-style
+    margin: -7px -15px;
+    padding: 7px 15px;
+  }
+
+  > details > summary {
+    display: inline-block;
+    list-style: none;
+    width: auto;
+    text-transform: uppercase;
+    font-size: 85%;
+    font-weight: 600;
+    // Override padding on ofn-drop-down-style to increase clickable area
+    margin: -8px -15px;
+    padding: 8px 15px;
+  }
+
+  > details > summary:after {
+    content: "\f0d7";
+    font-family: FontAwesome;
+  }
+
+  > details[open] > summary:after {
+    content: "\f0d8";
+    font-family: FontAwesome;
+  }
 }
 
 .ofn-drop-down-v2 {

app/webpacker/css/admin_v3/components/dropdown.scss

@@ -47,6 +47,7 @@
 
   &.disabled {
     opacity: 0.5;
+    pointer-events: none;
 
     &:hover {
       cursor: default;
@@ -179,6 +180,32 @@
       background-color: #ededed;
     }
   }
+
+  > details {
+    margin: -7px -15px;
+    padding: 7px 15px;
+  }
+
+  > details > summary {
+    display: inline-block;
+    list-style: none;
+    width: auto;
+    text-transform: uppercase;
+    font-size: 85%;
+    font-weight: 600;
+    margin: -8px -15px;
+    padding: 8px 15px;
+  }
+
+  > details > summary:after {
+    content: "\f0d7";
+    font-family: FontAwesome;
+  }
+
+  > details[open] > summary:after {
+    content: "\f0d8";
+    font-family: FontAwesome;
+  }
 }
 
 .ofn-drop-down-v2 {

spec/javascripts/stimulus/dropdown_controller_test.js

@@ -13,62 +13,36 @@ describe("Dropdown controller", () => {
 
   describe("Controller", () => {
     beforeEach(() => {
-      document.body.innerHTML = `<div data-controller="dropdown" id="container">
-        <span id="dropdown" data-action="click->dropdown#toggle">
-          <span id="arrow" data-dropdown-target="arrow" data-expanded-class="expandedClass expandedCLass2" data-collapsed-class="collapsedClass" />
-        </span>
-        <div id="menu" data-dropdown-target="menu" >
-
-        </div>
+      document.body.innerHTML = `<div id="container">
+        <details data-controller="dropdown" id="dropdown">
+          <summary id='summary'>
+            <span class="icon-reorder">
+            Actions
+            </span>
+          </summary>
+          <div id = "menu" class="menu" data-action="click->dropdown#closeOnMenu">
+            <div class="menu_item">
+              <span>Item 1</span>
+              <span>Item 2</span>
+            </div>
+          </div>
+        </details>
       </div>`;
     });
 
     afterEach(() => {
       document.body.innerHTML = "";
     });
 
-    it("hide menu by default", () => {
-      const menu = document.getElementById("menu");
-      expect(menu.classList.contains("hidden")).toBe(true);
-    });
-
-    it("show menu when toggle and add/remove class on arrow", () => {
-      const dropdown = document.getElementById("dropdown");
-      const arrow = document.getElementById("arrow");
-      const menu = document.getElementById("menu");
-      expect(menu.classList.contains("hidden")).toBe(true);
-      expect(arrow.classList.contains("expandedClass")).toBe(false);
-      expect(arrow.classList.contains("expandedClass2")).toBe(false);
-      expect(arrow.classList.contains("collapsedClass")).toBe(true);
-
-      dropdown.click();
-
-      expect(menu.classList.contains("hidden")).toBe(false);
-      expect(arrow.classList.contains("expandedClass")).toBe(true);
-      expect(arrow.classList.contains("expandedCLass2")).toBe(true);
-      expect(arrow.classList.contains("collapsedClass")).toBe(false);
-    });
-
     it ("hide menu when click outside", () => {
       const dropdown = document.getElementById("dropdown");
       const menu = document.getElementById("menu");
-      dropdown.click();
-      expect(menu.classList.contains("hidden")).toBe(false);
-
+      //open the details
+      dropdown.toggleAttribute('open') 
+      //click elsewhere
       document.body.click();
 
-      expect(menu.classList.contains("hidden")).toBe(true);
-    });
-
-    it ("do not display menu when disabled", () => {
-      const dropdown = document.getElementById("dropdown");
-      const container = document.getElementById("container");
-      const menu = document.getElementById("menu");
-      container.classList.add("disabled");
-
-      dropdown.click();
-
-      expect(menu.classList.contains("hidden")).toBe(true);
+      expect(dropdown.open).toBe(false);
     });
   });
 });

spec/system/admin/order_spec.rb

@@ -719,7 +719,7 @@ def new_order_with_distribution(distributor, order_cycle)
             it "should not display links but a js alert" do
               visit spree.edit_admin_order_path(order)
 
-              find("#links-dropdown .ofn-drop-down").click
+              find("summary", text: "ACTIONS").click
               expect(page).to have_link "Send Invoice", href: "#"
               expect(page).to have_link "Print Invoice", href: "#"
 
@@ -729,7 +729,7 @@ def new_order_with_distribution(distributor, order_cycle)
               expect(message)
                 .to eq "#{distributor1.name} must have a valid ABN before invoices can be used."
 
-              find("#links-dropdown .ofn-drop-down").click
+              find("summary", text: "ACTIONS").click
               message = accept_prompt do
                 click_link "Send Invoice"
               end

spec/system/admin/orders_spec.rb

@@ -497,8 +497,9 @@
         expect(page.find(
                  "#listing_orders tbody tr td:first-child input[type=checkbox]"
                )).to_not be_checked
-        # disables print invoices button
-        page.find("span.icon-reorder", text: "ACTIONS").click
+        # disables print invoices button not clickable
+        expect { find("span.icon-reorder", text: "ACTIONS").click }
+          .to raise_error(Capybara::Cuprite::MouseEventFailed)
         expect(page).to_not have_content "Print Invoices"
       end
     end