From af2d60cb698fe34de3c41bf0c5f178f2c7c7e5ea Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Thu, 30 Nov 2023 11:42:49 +0100 Subject: [PATCH] Refactor ingredient groups into details/summary These native components let us remove all the custom event handling and state management from the element editor. --- app/assets/stylesheets/alchemy/elements.scss | 16 +++----- .../components/element_editor.js | 41 +++++++++---------- .../alchemy/admin/elements/_element.html.erb | 12 +++--- .../admin/edit_elements_feature_spec.rb | 31 +------------- .../components/element_editor.spec.js | 24 ++++++----- 5 files changed, 44 insertions(+), 80 deletions(-) diff --git a/app/assets/stylesheets/alchemy/elements.scss b/app/assets/stylesheets/alchemy/elements.scss index adb8e4f6f3..9e242a5fa8 100644 --- a/app/assets/stylesheets/alchemy/elements.scss +++ b/app/assets/stylesheets/alchemy/elements.scss @@ -459,7 +459,7 @@ alchemy-tinymce { padding-bottom: 0; } - .ingredient-group-header { + summary { display: flex; align-items: center; justify-content: space-between; @@ -468,18 +468,14 @@ alchemy-tinymce { padding: $default-padding 1px; } - .ingredient-group-ingredients { - display: none; - } - - &.expanded { - .ingredient-group-ingredients { - display: block; - } - + &[open] { .ingredient-group-expand { @extend .fa-angle-up; } + + > :not(summary) { + box-sizing: border-box; + } } } diff --git a/app/javascript/alchemy_admin/components/element_editor.js b/app/javascript/alchemy_admin/components/element_editor.js index 33af475c15..4d48a04b7c 100644 --- a/app/javascript/alchemy_admin/components/element_editor.js +++ b/app/javascript/alchemy_admin/components/element_editor.js @@ -40,15 +40,6 @@ export class ElementEditor extends HTMLElement { this.toggle() } }) - on( - "click", - this.bodySelector, - "[data-toggle-ingredient-group]", - function (event) { - self.onToggleIngredientGroup(this) - event.preventDefault() - } - ) if (this.hasChildren) { this.addEventListener("alchemy:element-update-title", (event) => { @@ -69,6 +60,14 @@ export class ElementEditor extends HTMLElement { } if (this.body) { + this.body + .querySelectorAll(".ingredient-group") + .forEach((ingredientGroup) => { + ingredientGroup.addEventListener("toggle", () => { + this.onToggleIngredientGroup(ingredientGroup) + }) + }) + // We use of @rails/ujs for Rails remote forms this.body.addEventListener("ajax:success", (event) => { const responseJSON = event.detail[0] @@ -96,10 +95,11 @@ export class ElementEditor extends HTMLElement { const expanded_ingredient_groups = localStorage.getItem( "Alchemy.expanded_ingredient_groups" ) - Array.from(JSON.parse(expanded_ingredient_groups)).forEach((header_id) => { - const header = document.querySelector(`#${header_id}`) - const group = header?.closest(".ingredient-group") - group?.classList.add("expanded") + Array.from(JSON.parse(expanded_ingredient_groups)).forEach((group_id) => { + const group = document.querySelector(`#${group_id}`) + if (group) { + group.open = true + } }) } @@ -174,24 +174,21 @@ export class ElementEditor extends HTMLElement { /** * Toggle visibility of the ingredient fields in the group - * @param {HTMLLinkElement} target + * @param {HTMLLinkElement} group */ - onToggleIngredientGroup(target) { - const group_div = target.closest(".ingredient-group") - group_div.classList.toggle("expanded") - + onToggleIngredientGroup(group) { let expanded_ingredient_groups = JSON.parse( localStorage.getItem("Alchemy.expanded_ingredient_groups") || "[]" ) // Add or remove depending on whether this ingredient group is expanded - if (group_div.classList.contains("expanded")) { - if (expanded_ingredient_groups.indexOf(target.id) === -1) { - expanded_ingredient_groups.push(target.id) + if (group.open) { + if (expanded_ingredient_groups.indexOf(group.id) === -1) { + expanded_ingredient_groups.push(group.id) } } else { expanded_ingredient_groups = expanded_ingredient_groups.filter( - (value) => value !== target.id + (value) => value !== group.id ) } diff --git a/app/views/alchemy/admin/elements/_element.html.erb b/app/views/alchemy/admin/elements/_element.html.erb index 65c1708915..8d845bde1b 100644 --- a/app/views/alchemy/admin/elements/_element.html.erb +++ b/app/views/alchemy/admin/elements/_element.html.erb @@ -37,15 +37,13 @@ <% element.ingredients.select { |i| i.definition[:group] }.group_by { |i| i.definition[:group] }.each do |group, ingredients| %> -
- <%= link_to '#', id: "element_#{element.id}_ingredient_group_#{group.parameterize.underscore}_header", class: 'ingredient-group-header', data: { toggle_ingredient_group: true } do %> + <%= content_tag :details, class: "ingredient-group", id: "element_#{element.id}_ingredient_group_#{group.parameterize.underscore}" do %> + <%= element.translated_group group %> - <% end %> - <%= content_tag :div, id: "element_#{element.id}_ingredient_group_#{group.parameterize.underscore}", class: 'ingredient-group-ingredients' do %> - <%= render ingredients, element_form: f %> - <% end %> -
+ + <%= render ingredients, element_form: f %> + <% end %> <% end %> <% end %> diff --git a/spec/features/admin/edit_elements_feature_spec.rb b/spec/features/admin/edit_elements_feature_spec.rb index ef81472ce2..1ab85d0df7 100644 --- a/spec/features/admin/edit_elements_feature_spec.rb +++ b/spec/features/admin/edit_elements_feature_spec.rb @@ -147,38 +147,9 @@ # Need to be on page editor rather than just admin_elements in order to have JS interaction before { visit alchemy.edit_admin_page_path(element.page) } - scenario "collapsed ingredient groups shown", :js do - # No group ingredient initially visible - expect(page).not_to have_selector(".ingredient-group-ingredients", visible: true) - - page.find("a#element_#{element.id}_ingredient_group_details_header", text: "Details").click - # 'Details' group ingredient visible - expect(page).to have_selector("#element_#{element.id}_ingredient_group_details", visible: true) - within("#element_#{element.id}_ingredient_group_details") do - expect(page).to have_selector("[data-ingredient-role='description']") - expect(page).to have_selector("[data-ingredient-role='key_words']") - end - expect(page).to have_selector("#element_#{element.id}_ingredient_group_details", visible: true) - - # 'Size' group ingredient not visible - expect(page).not_to have_selector("#element_#{element.id}_ingredient_group_size", visible: true) - - page.find("a#element_#{element.id}_ingredient_group_size_header", text: "Size").click - # 'Size' group now visible - expect(page).to have_selector("#element_#{element.id}_ingredient_group_size", visible: true) - within("#element_#{element.id}_ingredient_group_size") do - expect(page).to have_selector("[data-ingredient-role='width']") - expect(page).to have_selector("[data-ingredient-role='height']") - end - - page.find("a#element_#{element.id}_ingredient_group_size_header", text: "Size").click - # 'Size' group hidden - expect(page).not_to have_selector("#element_#{element.id}_ingredient_group_size", visible: true) - end - scenario "expanded ingredient groups persist between visits", :js do expect(page).not_to have_selector("#element_#{element.id}_ingredient_group_details", visible: true) - page.find("a#element_#{element.id}_ingredient_group_details_header", text: "Details").click + page.find("details#element_#{element.id}_ingredient_group_details", text: "Details").click expect(page).to have_selector("#element_#{element.id}_ingredient_group_details", visible: true) visit alchemy.edit_admin_page_path(element.page) expect(page).to have_selector("#element_#{element.id}_ingredient_group_details", visible: true) diff --git a/spec/javascript/alchemy_admin/components/element_editor.spec.js b/spec/javascript/alchemy_admin/components/element_editor.spec.js index 50fdad7089..f0fcd220f9 100644 --- a/spec/javascript/alchemy_admin/components/element_editor.spec.js +++ b/spec/javascript/alchemy_admin/components/element_editor.spec.js @@ -212,24 +212,26 @@ describe("alchemy-element-editor", () => { }) describe("on click on ingredient group header", () => { - it("expands ingredient group", () => { + it("stores ingredient group state in localStorage", () => { editor = getComponent(`
-
- - Details - +
+ Details
-
+
`) - const header = editor.querySelector(".ingredient-group-header") - const click = new Event("click", { bubbles: true }) - header.dispatchEvent(click) - const group = editor.querySelector(".ingredient-group") - expect(group.classList.contains("expanded")).toBeTruthy() + const group = editor.querySelector("details") + expect( + localStorage.hasOwnProperty("Alchemy.expanded_ingredient_groups") + ).toBeFalsy() + const click = new Event("toggle", { bubbles: true }) + group.dispatchEvent(click) + expect( + localStorage.hasOwnProperty("Alchemy.expanded_ingredient_groups") + ).toBeTruthy() localStorage.clear() }) })