Merge pull request #14085 from dacook/admin-product-actions-fixes

[Admin Products] Action menu fixes

app/components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.scss

@@ -19,7 +19,9 @@
     background-color: white;
     box-shadow: $box-shadow;
     border-radius: 3px;
+    width: max-content;
     min-width: 80px;
+    max-width: 110px;
     display: none;
     z-index: 100;
 
@@ -27,6 +29,15 @@
       display: block;
     }
 
+    // Fade out so user can see which option was selected
+    &.selected {
+      transition:
+        opacity 0.2s linear,
+        visibility 0.2s linear;
+      opacity: 0;
+      visibility: hidden;
+    }
+
     & > a {
       display: block;
       padding: 5px 10px;

app/components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_controller.js

@@ -6,6 +6,9 @@ export default class extends Controller {
   connect() {
     super.connect();
     window.addEventListener("click", this.#hideIfClickedOutside);
+
+    // Close menu when making a selection
+    this.contentTarget.addEventListener("click", this.#selected);
   }
 
   disconnect() {
@@ -13,17 +16,22 @@ export default class extends Controller {
   }
 
   toggle() {
-    this.contentTarget.classList.toggle("show");
+    this.#toggleShow();
   }
 
+  #selected = () => {
+    this.contentTarget.classList.add("selected");
+  };
+
   #hideIfClickedOutside = (event) => {
     if (this.element.contains(event.target)) {
       return;
     }
-    this.#hide();
+    this.#toggleShow(false);
   };
 
-  #hide() {
-    this.contentTarget.classList.remove("show");
+  #toggleShow(force = undefined) {
+    this.contentTarget.classList.toggle("show", force);
+    this.contentTarget.classList.remove("selected");
   }
 }

app/views/admin/products_v3/_variant_row.html.haml

@@ -4,7 +4,7 @@
 %td.col-image
   -# empty
   - variant.source_variants.each do |source_variant|
-    = content_tag(:span, "🔗", title: t('admin.products_page.variant_row.sourced_from', source_name: source_variant.display_name, source_id: source_variant.id, hub_name: variant.hub&.name))
+    = content_tag(:span, "🔗", title: t('admin.products_page.variant_row.sourced_from', source_name: source_variant.full_name, source_id: source_variant.id, hub_name: variant.hub&.name))
 %td.col-name.field.naked_inputs
   = f.hidden_field :id
   = f.text_field :display_name, 'aria-label': t('admin.products_page.columns.name'), placeholder: variant.product.name

app/webpacker/css/admin/products_v3.scss

@@ -375,6 +375,6 @@
 
   .slide-in {
     transform-origin: top;
-    animation: slideInTop 0.5s forwards;
+    animation: slideInTop 0.5s;
   }
 }

script/reviewdog.sh

@@ -6,7 +6,7 @@
 
 set -o pipefail
 
-echo "::group:: Running prettier with reviewdog 🐶 ..."
+echo -e "\nRunning prettier with reviewdog 🐶 ..."
 
 "$(npm root)/.bin/prettier" --check . 2>&1 | sed --regexp-extended 's/(\[warn\].*)$/\1 File is not properly formatted./' \
   | reviewdog \
@@ -25,7 +25,7 @@ echo "::group:: Running prettier with reviewdog 🐶 ..."
 
 prettier=$?
 
-echo "::group:: Running rubocop with reviewdog 🐶 ..."
+echo -e "\nRunning rubocop with reviewdog 🐶 ..."
 
 bundle exec rubocop \
   --fail-level info \
@@ -39,7 +39,7 @@ bundle exec rubocop \
 
 rubocop=$?
 
-echo "::group:: Running haml-lint with reviewdog 🐶 ..."
+echo -e "\nRunning haml-lint with reviewdog 🐶 ..."
 
 bundle exec haml-lint \
   --fail-level warning \

spec/javascripts/stimulus/vertical_ellipsis_menu_controller_test.js

@@ -16,12 +16,13 @@ describe("VerticalEllipsisMenuController test", () => {
       <div data-controller="vertical-ellipsis-menu" id="root">
         <div data-action="click->vertical-ellipsis-menu#toggle" id="button">...</div>
         <div data-vertical-ellipsis-menu-target="content" id="content">
-          
+          <a href="#" id="item"></a>
         </div>
       </div>
     `;
     const button = document.getElementById("button");
     const content = document.getElementById("content");
+    const item = document.getElementById("item");
   });
 
   it("add show class to content when toggle is called", () => {
@@ -43,4 +44,15 @@ describe("VerticalEllipsisMenuController test", () => {
     document.body.click();
     expect(content.classList.contains("show")).toBe(false);
   });
+
+  it("adds selected class to content when clicking a menu item", () => {
+    button.click();
+    expect(content.classList.contains("selected")).toBe(false);
+    item.click();
+    expect(content.classList.contains("selected")).toBe(true);
+
+    // and removes it again when clicking button again
+    button.click();
+    expect(content.classList.contains("selected")).toBe(false);
+  });
 });

spec/system/admin/products_v3/actions_spec.rb

@@ -295,12 +295,11 @@ RSpec.describe 'As an enterprise user, I can perform actions on the products scr
             # Close action menu (shouldn't need this, it should close itself)
             last_box.click
 
-            # And I can perform actions on the new product
+            # And I can perform actions on the new variant
             within last_box do
               page.find(".vertical-ellipsis-menu").click
               expect(page).to have_link "Edit"
-              # expect(page).to have_link "Clone" # tofix: menu is partially obscured
-              # expect(page).to have_link "Delete" # it's not a proper link
+              expect(page).to have_selector "a", text: "Delete" # it's not a proper link
 
               fill_in "Name", with: "My copy of Apples"
             end
@@ -339,12 +338,10 @@ RSpec.describe 'As an enterprise user, I can perform actions on the products scr
       }
 
       describe "Actions columns (delete)" do
-        before do
-          visit admin_products_url
-        end
-
         it "shows an actions menu with a delete link when clicking on icon for product. " \
            "doesn't show delete link for the single variant" do
+          visit admin_products_url
+
           within product_selector do
             page.find(".vertical-ellipsis-menu").click
             expect(page).to have_css(delete_option_selector)