From 120e2996534dbfaeb75681cd623443c3049f51d4 Mon Sep 17 00:00:00 2001 From: Dusan Orlovic Date: Tue, 14 Nov 2023 07:40:07 +0100 Subject: [PATCH] Use activate instead of changeActivePanel/ChangeActiveTab Remove shop-tabs controllers since we can listen on `"data-action": "orderCycleSelected@window->tabs-and-panels#activateDefaultPanel"` Test for cases: * activate by clicking on tab * activateDefaultPanel on orderCycleSelected event * activateFromWindowLocationOrDefaultPanelTarget to activate tab based on achor in URL --- .../templates/partials/hub_details.html.haml | 2 +- .../partials/producer_details.html.haml | 2 +- app/helpers/shared_helper.rb | 2 +- app/views/admin/shared/_side_menu.html.haml | 4 +- .../messages/_customer_required.html.haml | 4 +- app/views/shopping_shared/_tabs.html.haml | 4 +- .../orders/form/_update_buttons.html.haml | 2 +- .../controllers/shop_tabs_controller.js | 22 -- .../controllers/tabs_and_panels_controller.js | 105 +++------ .../tabs_and_panels_controller_test.js | 213 ++++++++++-------- spec/system/admin/enterprises_spec.rb | 28 +-- 11 files changed, 170 insertions(+), 218 deletions(-) delete mode 100644 app/webpacker/controllers/shop_tabs_controller.js diff --git a/app/assets/javascripts/templates/partials/hub_details.html.haml b/app/assets/javascripts/templates/partials/hub_details.html.haml index 9ecaef9493..d9fb935f2a 100644 --- a/app/assets/javascripts/templates/partials/hub_details.html.haml +++ b/app/assets/javascripts/templates/partials/hub_details.html.haml @@ -14,7 +14,7 @@ {{'hubs_delivery' | t}} .row .columns.small-12 - %a.cta-hub{"ng-href" => "{{::enterprise.path}}#/shop", "ng-attr-target" => "{{ embedded_layout ? '_blank' : undefined}}", + %a.cta-hub{"ng-href" => "{{::enterprise.path}}#/shop_panel", "ng-attr-target" => "{{ embedded_layout ? '_blank' : undefined}}", "ng-class" => "{primary: enterprise.active, secondary: !enterprise.active}", "ng-click" => "$close()", "ofn-change-hub" => "enterprise"} diff --git a/app/assets/javascripts/templates/partials/producer_details.html.haml b/app/assets/javascripts/templates/partials/producer_details.html.haml index 0e4bf23513..814e55b241 100644 --- a/app/assets/javascripts/templates/partials/producer_details.html.haml +++ b/app/assets/javascripts/templates/partials/producer_details.html.haml @@ -12,7 +12,7 @@ .row .columns.small-12 %a.cta-hub{"ng-repeat" => "hub in enterprise.hubs | filter:{id: '!'+enterprise.id} | orderBy:'-active'", - "ng-href" => "{{::hub.path}}#/shop", "ofn-empties-cart" => "hub", + "ng-href" => "{{::hub.path}}#/shop_panel", "ofn-empties-cart" => "hub", "ng-class" => "::{primary: hub.active, secondary: !hub.active}", "ng-click" => "$close()", "ofn-change-hub" => "hub"} diff --git a/app/helpers/shared_helper.rb b/app/helpers/shared_helper.rb index b1f51ab004..17bd12adbe 100644 --- a/app/helpers/shared_helper.rb +++ b/app/helpers/shared_helper.rb @@ -20,6 +20,6 @@ module SharedHelper end def current_shop_products_path - "#{main_app.enterprise_shop_path(current_distributor)}#/shop" + "#{main_app.enterprise_shop_path(current_distributor)}#/shop_panel" end end diff --git a/app/views/admin/shared/_side_menu.html.haml b/app/views/admin/shared/_side_menu.html.haml index 6240519f28..168add17ce 100644 --- a/app/views/admin/shared/_side_menu.html.haml +++ b/app/views/admin/shared/_side_menu.html.haml @@ -2,12 +2,12 @@ - if @enterprise - enterprise_side_menu_items(@enterprise).each do |item| - next if !item[:show] || (item[:name] == 'vouchers' && !feature?(:vouchers, spree_current_user, @enterprise)) - %a.menu_item{ href: item[:href] || "##{item[:name]}_panel", id: item[:name], data: { action: "tabs-and-panels#changeActivePanel tabs-and-panels#changeActiveTab", "tabs-and-panels-target": "tab" }, class: item[:selected] } + %a.menu_item{ href: item[:href] || "##{item[:name]}_panel", data: { action: "tabs-and-panels#activate", "tabs-and-panels-target": "tab", test: "link_for_#{item[:name]}" }, class: item[:selected] } %i{ class: item[:icon_class] } %span= t(".enterprise.#{item[:name] }") - else - enterprise_group_side_menu_items.each do |item| - %a.menu_item{ href: "##{item[:name]}_panel", class: item[:selected], id: item[:name], data: { action: "tabs-and-panels#changeActivePanel tabs-and-panels#changeActiveTab", "tabs-and-panels-target": "tab" } } + %a.menu_item{ href: "##{item[:name]}_panel", class: item[:selected], data: { action: "tabs-and-panels#activate", "tabs-and-panels-target": "tab", test: "link_for_#{item[:name]}" } } %i{ class: item[:icon_class] } %span= t(".enterprise_group.#{item[:name] }") diff --git a/app/views/shop/messages/_customer_required.html.haml b/app/views/shop/messages/_customer_required.html.haml index f5f9f9f5c8..844621ce28 100644 --- a/app/views/shop/messages/_customer_required.html.haml +++ b/app/views/shop/messages/_customer_required.html.haml @@ -8,6 +8,6 @@ %p = t('.require_login_link_html', login: ('' + t('.login') + '').html_safe) %p - = t('.require_login_2_html', contact: link_to(t('.contact'), '#contact_panel', data: { action: "tabs-and-panels#changeActivePanel tabs-and-panels#changeActiveTab", "tabs-and-panels-target": "tab" }, id: "contact"), enterprise: current_distributor.name) + = t('.require_login_2_html', contact: link_to(t('.contact'), '#contact_panel', data: { action: "tabs-and-panels#activate" }), enterprise: current_distributor.name) - else - = t('.require_customer_html', contact: link_to(t('.contact'), '#contact_panel', data: { action: "tabs-and-panels#changeActivePanel tabs-and-panels#changeActiveTab", "tabs-and-panels-target": "tab" }, id: "contact"), enterprise: current_distributor.name) + = t('.require_customer_html', contact: link_to(t('.contact'), '#contact_panel', data: { action: "tabs-and-panels#activate" }), enterprise: current_distributor.name) diff --git a/app/views/shopping_shared/_tabs.html.haml b/app/views/shopping_shared/_tabs.html.haml index 8497416f93..b1d572f5bd 100644 --- a/app/views/shopping_shared/_tabs.html.haml +++ b/app/views/shopping_shared/_tabs.html.haml @@ -1,12 +1,12 @@ - if (@order&.distributor || current_distributor) == current_distributor - #shop-tabs{"data-controller": "tabs-and-panels shop-tabs", "data-tabs-and-panels-class-name-value": "selected"} + #shop-tabs{"data-controller": "tabs-and-panels", "data-action": "orderCycleSelected@window->tabs-and-panels#activateDefaultPanel", "data-tabs-and-panels-class-name-value": "selected"} .tab-buttons .flex.row .columns.small-12.large-8 - shop_tabs.each do |tab| .page - %a{ id: tab[:name], href: "##{tab[:name]}_panel", data: { action: "tabs-and-panels#changeActivePanel tabs-and-panels#changeActiveTab", "tabs-and-panels-target": "tab" }, class: ("selected" if tab[:default]) }=tab[:title] + %a{ href: "##{tab[:name]}_panel", data: { action: "tabs-and-panels#activate", "tabs-and-panels-target": "tab" }, class: ("selected" if tab[:default]) }=tab[:title] .columns.large-4.show-for-large-up = render partial: "shopping_shared/order_cycles" - shop_tabs.each do |tab| diff --git a/app/views/spree/orders/form/_update_buttons.html.haml b/app/views/spree/orders/form/_update_buttons.html.haml index 35b9ceaee4..50e256e540 100644 --- a/app/views/spree/orders/form/_update_buttons.html.haml +++ b/app/views/spree/orders/form/_update_buttons.html.haml @@ -6,7 +6,7 @@ = t(:order_back_to_cart) - else .columns.small-12.medium-6 - = link_to "#{main_app.enterprise_shop_path(@order.distributor)}#/shop", class: "button expand" do + = link_to "#{main_app.enterprise_shop_path(@order.distributor)}#/shop_panel", class: "button expand" do = t(:order_back_to_store) .columns.small-12.medium-6 - if @order.distributor.website.present? diff --git a/app/webpacker/controllers/shop_tabs_controller.js b/app/webpacker/controllers/shop_tabs_controller.js deleted file mode 100644 index 86001f9570..0000000000 --- a/app/webpacker/controllers/shop_tabs_controller.js +++ /dev/null @@ -1,22 +0,0 @@ -import { Controller } from "stimulus"; - -export default class extends Controller { - connect() { - window.addEventListener("orderCycleSelected", this.orderCycleSelected); - } - - disconnect() { - window.removeEventListener("orderCycleSelected", this.orderCycleSelected); - } - - orderCycleSelected = (event) => { - window.dispatchEvent( - new CustomEvent("tabs-and-panels:click", { - detail: { - tab: "shop", - panel: "shop_panel", - }, - }) - ); - }; -} diff --git a/app/webpacker/controllers/tabs_and_panels_controller.js b/app/webpacker/controllers/tabs_and_panels_controller.js index 050a4fff25..33151453e6 100644 --- a/app/webpacker/controllers/tabs_and_panels_controller.js +++ b/app/webpacker/controllers/tabs_and_panels_controller.js @@ -5,93 +5,46 @@ export default class extends Controller { static values = { className: String }; connect() { - // hide all active panel - this.panelTargets.forEach((panel) => { - panel.style.display = "none"; - }); - - // only display the default panel - this.defaultTarget.style.display = "block"; - - // Display panel specified in url anchor - const anchors = window.location.toString().split("#"); - let anchor = anchors.length > 1 ? anchors.pop() : ""; - - if (anchor != "") { - // Conveniently AngularJs rewrite "example.com#panel" to "example.com#/panel" :( - // strip the starting / if any - if (anchor[0] == "/") { - anchor = anchor.slice(1); - } - // Add _panel to the anchor to match the panel id if needed - if (!anchor.includes("_panel")) { - anchor = `${anchor}_panel`; - } - this.updateActivePanel(anchor); - - // tab - const tab_id = anchor.split("_panel").shift(); - this.updateActiveTab(tab_id); - } - - window.addEventListener("tabs-and-panels:click", (event) => { - this.simulateClick(event.detail.tab, event.detail.panel); - }); + this._activateFromWindowLocationOrDefaultPanelTarget(); window.addEventListener("popstate", (event) => { - const newPanelId = event.target.location.hash.replace("#/", ""); - const currentPanelId = this.currentActivePanel.id; - - if (newPanelId !== currentPanelId) { - const newTabId = newPanelId.split("_panel").shift(); - this.simulateClick(newTabId, newPanelId); - } + this._activateFromWindowLocationOrDefaultPanelTarget(); }); } - simulateClick(tab, panel) { - this.updateActivePanel(panel); - this.updateActiveTab(tab); - } - - changeActivePanel(event) { - this.updateActivePanel(`${event.currentTarget.id}_panel`); - } - - updateActivePanel(panel_id) { - const newActivePanel = this.panelTargets.find((panel) => panel.id == panel_id); - - if (newActivePanel === undefined) { - // No panel found - return; + _activateFromWindowLocationOrDefaultPanelTarget() { + // Conveniently AngularJs rewrite "example.com#panel" to "example.com#/panel" + const hashWithoutSlash = window.location.hash.replace("/", ""); + const tabWithSameHash = this.tabTargets.find((tab) => tab.hash == hashWithoutSlash); + if (hashWithoutSlash != "" && tabWithSameHash) { + this._activateByHash(tabWithSameHash.hash); + } else { + this._activateByHash(`#${this.defaultTarget.id}`); } - - this.currentActivePanel.style.display = "none"; - newActivePanel.style.display = "block"; } - changeActiveTab(event) { - this.currentActiveTab.classList.remove(`${this.classNameValue}`); - event.currentTarget.classList.add(`${this.classNameValue}`); + activate(event) { + this._activateByHash(event.currentTarget.hash); } - updateActiveTab(tab_id) { - const newActiveTab = this.tabTargets.find((tab) => tab.id == tab_id); - - if (newActiveTab === undefined) { - // No tab found - return; - } - - this.currentActiveTab.classList.remove(`${this.classNameValue}`); - newActiveTab.classList.add(`${this.classNameValue}`); + activateDefaultPanel() { + this._activateByHash(`#${this.defaultTarget.id}`); } - get currentActiveTab() { - return this.tabTargets.find((tab) => tab.classList.contains("selected")); - } - - get currentActivePanel() { - return this.panelTargets.find((panel) => panel.id == `${this.currentActiveTab.id}_panel`); + _activateByHash(hash) { + this.tabTargets.forEach((tab) => { + if (tab.hash == hash) { + tab.classList.add(this.classNameValue); + } else { + tab.classList.remove(this.classNameValue); + } + }); + this.panelTargets.forEach((panel) => { + if (panel.id == hash.replace("#", "")) { + panel.style.display = "block"; + } else { + panel.style.display = "none"; + } + }); } } diff --git a/spec/javascripts/stimulus/tabs_and_panels_controller_test.js b/spec/javascripts/stimulus/tabs_and_panels_controller_test.js index 523f5eb04f..c64421923d 100644 --- a/spec/javascripts/stimulus/tabs_and_panels_controller_test.js +++ b/spec/javascripts/stimulus/tabs_and_panels_controller_test.js @@ -11,122 +11,143 @@ describe('TabsAndPanelsController', () => { application.register('tabs-and-panels', tabs_and_panels_controller); }); - describe('#tabs-and-panels', () => { - const checkDefaultPanel = () => { - const peekPanel = document.getElementById('peek_panel'); - const kaPanel = document.getElementById('ka_panel'); - const booPanel = document.getElementById('boo_panel'); + beforeEach(() => { + document.body.innerHTML = ` +
+ Peek + Ka + Boo - expect(peekPanel.style.display).toBe('block'); - expect(kaPanel.style.display).toBe('none'); - expect(booPanel.style.display).toBe('none'); - } +
Peek me
+
Ka you
+
Boo three
+
` + }) - beforeEach(() => { - document.body.innerHTML = ` -
- Peek - Ka - Boo + it("#activate by clicking on tab", () => { + const peakTab = document.getElementById("peek_tab") + const kaTab = document.getElementById("ka_tab") + const booTab = document.getElementById("boo_tab") + const peakPanel = document.getElementById("peek_panel") + const kaPanel = document.getElementById("ka_panel") + const booPanel = document.getElementById("boo_panel") + expect(peakTab.classList.contains("selected")).toBe(true) + expect(kaTab.classList.contains("selected")).toBe(false) + expect(booTab.classList.contains("selected")).toBe(false) -
Peek me
-
Ka you
-
Boo three
-
`; - }); + expect(peakPanel.style.display).toBe("block") + expect(kaPanel.style.display).toBe("none") + expect(booPanel.style.display).toBe("none") - it('displays only the default panel', () => { - checkDefaultPanel() - }); + kaTab.click() - describe('when tab is clicked', () => { - let ka; + expect(peakTab.classList.contains("selected")).toBe(false) + expect(kaTab.classList.contains("selected")).toBe(true) + expect(booTab.classList.contains("selected")).toBe(false) - beforeEach(() => { - ka = document.getElementById('ka'); - }) + expect(peakPanel.style.display).toBe("none") + expect(kaPanel.style.display).toBe("block") + expect(booPanel.style.display).toBe("none") + }) - it('displays appropriate panel', () => { - const kaPanel = document.getElementById('ka_panel'); + it("#activateDefaultPanel on orderCycleSelected event", () => { + const peakTab = document.getElementById("peek_tab") + const kaTab = document.getElementById("ka_tab") + const booTab = document.getElementById("boo_tab") + const peakPanel = document.getElementById("peek_panel") + const kaPanel = document.getElementById("ka_panel") + const booPanel = document.getElementById("boo_panel") - expect(kaPanel.style.display).toBe('none'); - ka.click(); - expect(kaPanel.style.display).toBe('block'); - }); + expect(peakTab.classList.contains("selected")).toBe(true) + expect(kaTab.classList.contains("selected")).toBe(false) + expect(booTab.classList.contains("selected")).toBe(false) - it('selects the clicked tab', () => { - ka.click(); - expect(ka.classList.contains('selected')).toBe(true); - }); + expect(peakPanel.style.display).toBe("block") + expect(kaPanel.style.display).toBe("none") + expect(booPanel.style.display).toBe("none") - describe("when panel doesn't exist", () => { - beforeEach(() => { - document.body.innerHTML = ` -
- Peek - Ka - Boo + kaTab.click() + expect(peakTab.classList.contains("selected")).toBe(false) + expect(kaTab.classList.contains("selected")).toBe(true) + expect(booTab.classList.contains("selected")).toBe(false) -
Peek me
-
Boo three
-
`; - }); + expect(peakPanel.style.display).toBe("none") + expect(kaPanel.style.display).toBe("block") + expect(booPanel.style.display).toBe("none") - it('displays the current panel', () => { - const peekPanel = document.getElementById('peek_panel'); + const event = new Event("orderCycleSelected") + window.dispatchEvent(event); - ka.click(); - expect(peekPanel.style.display).toBe('block'); - }) + expect(peakTab.classList.contains("selected")).toBe(true) + expect(kaTab.classList.contains("selected")).toBe(false) + expect(booTab.classList.contains("selected")).toBe(false) + + expect(peakPanel.style.display).toBe("block") + expect(kaPanel.style.display).toBe("none") + expect(booPanel.style.display).toBe("none") + }) + + describe("when valid anchor is specified in the url", () => { + const oldWindowLocation = window.location + beforeAll(() => { + Object.defineProperty(window, "location", { + value: new URL("http://example.com/#boo_panel"), + configurable: true, }) }) + afterAll(() => { + delete window.location + window.location = oldWindowLocation + }) - describe('when anchor is specified in the url', () => { - const { location } = window; - const mockLocationToString = (panel) => { - // Mocking window.location.toString() - const url = `http://localhost:3000/admin/enterprises/great-shop/edit#/${panel}` - const mockedToString = jest.fn() - mockedToString.mockImplementation(() => (url)) + it("#activateFromWindowLocationOrDefaultPanelTarget show panel based on anchor", () => { + const peakTab = document.getElementById("peek_tab") + const kaTab = document.getElementById("ka_tab") + const booTab = document.getElementById("boo_tab") + const peakPanel = document.getElementById("peek_panel") + const kaPanel = document.getElementById("ka_panel") + const booPanel = document.getElementById("boo_panel") - delete window.location - window.location = { - toString: mockedToString - } - } + expect(peakTab.classList.contains("selected")).toBe(false) + expect(kaTab.classList.contains("selected")).toBe(false) + expect(booTab.classList.contains("selected")).toBe(true) - beforeAll(() => { - mockLocationToString('ka_panel') - }) + expect(peakPanel.style.display).toBe("none") + expect(kaPanel.style.display).toBe("none") + expect(booPanel.style.display).toBe("block") + }) + }) - afterAll(() => { - // cleaning up - window.location = location - }) - - it('displays the panel associated with the anchor', () => { - const kaPanel = document.getElementById('ka_panel'); - - expect(kaPanel.style.display).toBe('block'); - }) - - it('selects the tab entry associated with the anchor', () => { - const ka = document.getElementById('ka'); - - expect(ka.classList.contains('selected')).toBe(true); - }) - - describe("when anchor doesn't macht any panel", () => { - beforeAll(() => { - mockLocationToString('random_panel') - }) - - it('displays the default panel', () => { - checkDefaultPanel() - }) + describe("when non valid anchor is specified in the url", () => { + const oldWindowLocation = window.location + beforeAll(() => { + Object.defineProperty(window, "location", { + value: new URL("http://example.com/#non_valid_panel"), + configurable: true, }) }) - }); -}); + afterAll(() => { + delete window.location + window.location = oldWindowLocation + }) + + it("#activateFromWindowLocationOrDefaultPanelTarget show default panel", () => { + const peakTab = document.getElementById("peek_tab") + const kaTab = document.getElementById("ka_tab") + const booTab = document.getElementById("boo_tab") + const peakPanel = document.getElementById("peek_panel") + const kaPanel = document.getElementById("ka_panel") + const booPanel = document.getElementById("boo_panel") + + expect(peakTab.classList.contains("selected")).toBe(true) + expect(kaTab.classList.contains("selected")).toBe(false) + expect(booTab.classList.contains("selected")).toBe(false) + + expect(peakPanel.style.display).toBe("block") + expect(kaPanel.style.display).toBe("none") + expect(booPanel.style.display).toBe("none") + }) + }) +}) diff --git a/spec/system/admin/enterprises_spec.rb b/spec/system/admin/enterprises_spec.rb index 1660fc3da2..a6b4cdbb8a 100644 --- a/spec/system/admin/enterprises_spec.rb +++ b/spec/system/admin/enterprises_spec.rb @@ -131,25 +131,25 @@ describe ' # Unchecking hides the Properties tab uncheck 'enterprise_is_primary_producer' choose 'None' - expect(page).not_to have_selector "#enterprise_fees" - expect(page).not_to have_selector "#payment_methods" - expect(page).not_to have_selector "#shipping_methods" - expect(page).not_to have_selector "#properties" + expect(page).not_to have_selector "[data-test=link_for_enterprise_fees]" + expect(page).not_to have_selector "[data-test=link_for_payment_methods]" + expect(page).not_to have_selector "[data-test=link_for_shipping_methods]" + expect(page).not_to have_selector "[data-test=link_for_properties]" # Checking displays the Properties tab check 'enterprise_is_primary_producer' - expect(page).to have_selector "#enterprise_fees" - expect(page).not_to have_selector "#payment_methods" - expect(page).not_to have_selector "#shipping_methods" - expect(page).to have_selector "#properties" + expect(page).to have_selector "[data-test=link_for_enterprise_fees]" + expect(page).not_to have_selector "[data-test=link_for_payment_methods]" + expect(page).not_to have_selector "[data-test=link_for_shipping_methods]" + expect(page).to have_selector "[data-test=link_for_properties]" uncheck 'enterprise_is_primary_producer' choose 'Own' - expect(page).to have_selector "#enterprise_fees" - expect(page).to have_selector "#payment_methods" - expect(page).to have_selector "#shipping_methods" + expect(page).to have_selector "[data-test=link_for_enterprise_fees]" + expect(page).to have_selector "[data-test=link_for_payment_methods]" + expect(page).to have_selector "[data-test=link_for_shipping_methods]" choose 'Any' - expect(page).to have_selector "#enterprise_fees" - expect(page).to have_selector "#payment_methods" - expect(page).to have_selector "#shipping_methods" + expect(page).to have_selector "[data-test=link_for_enterprise_fees]" + expect(page).to have_selector "[data-test=link_for_payment_methods]" + expect(page).to have_selector "[data-test=link_for_shipping_methods]" page.find("#enterprise_group_ids-ts-control").set(eg1.name) page.find("#enterprise_group_ids-ts-dropdown .option.active").click