From ed93c77578e3d4ad28ad8f701001da788dda97d1 Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 30 Nov 2023 11:29:20 +1100 Subject: [PATCH 1/4] Load Stimulus controllers from 'sidecar' directories Javascript files can now be included in the component directory, alongside HTML and CSS. As per https://viewcomponent.org/guide/javascript_and_css.html --- app/webpacker/controllers/index.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/webpacker/controllers/index.js b/app/webpacker/controllers/index.js index 64b87d935e..00245fdf88 100644 --- a/app/webpacker/controllers/index.js +++ b/app/webpacker/controllers/index.js @@ -9,7 +9,9 @@ import CableReady from "cable_ready"; const application = Application.start(); const context = require.context("controllers", true, /_controller\.js$/); -application.load(definitionsFromContext(context)); +const contextComponents = require.context("../../components", true, /_controller\.js$/); + +application.load(definitionsFromContext(context).concat(definitionsFromContext(contextComponents))); application.consumer = consumer; StimulusReflex.initialize(application, { controller, isolate: true }); StimulusReflex.debug = process.env.RAILS_ENV === "development"; From f6d72127a7f0cfbdb4b65607cf4fa944812a109c Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 30 Nov 2023 11:54:29 +1100 Subject: [PATCH 2/4] Move controller to sidecar directory --- .../component_controller.js} | 0 .../vertical_ellipsis_menu_component.html.haml | 6 +++--- .../stimulus/vertical_ellipsis_menu_controller_test.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) rename app/{webpacker/controllers/vertical_ellipsis_menu_controller.js => components/vertical_ellipsis_menu_component/component_controller.js} (100%) diff --git a/app/webpacker/controllers/vertical_ellipsis_menu_controller.js b/app/components/vertical_ellipsis_menu_component/component_controller.js similarity index 100% rename from app/webpacker/controllers/vertical_ellipsis_menu_controller.js rename to app/components/vertical_ellipsis_menu_component/component_controller.js diff --git a/app/components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.html.haml b/app/components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.html.haml index ea67c8ef00..5557289769 100644 --- a/app/components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.html.haml +++ b/app/components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.html.haml @@ -1,4 +1,4 @@ -.vertical-ellipsis-menu{ "data-controller": "vertical-ellipsis-menu" } - %i.fa.fa-ellipsis-v{ "data-action": "click->vertical-ellipsis-menu#toggle" } - .vertical-ellipsis-menu-content{ "data-vertical-ellipsis-menu-target": "content" } +.vertical-ellipsis-menu{ "data-controller": "vertical-ellipsis-menu--component" } + %i.fa.fa-ellipsis-v{ "data-action": "click->vertical-ellipsis-menu--component#toggle" } + .vertical-ellipsis-menu-content{ "data-vertical-ellipsis-menu--component-target": "content" } = content diff --git a/spec/javascripts/stimulus/vertical_ellipsis_menu_controller_test.js b/spec/javascripts/stimulus/vertical_ellipsis_menu_controller_test.js index da53c53a14..7e9d830c6b 100644 --- a/spec/javascripts/stimulus/vertical_ellipsis_menu_controller_test.js +++ b/spec/javascripts/stimulus/vertical_ellipsis_menu_controller_test.js @@ -3,7 +3,7 @@ */ import { Application } from "stimulus"; -import vertical_ellipsis_menu_controller from "../../../app/webpacker/controllers/vertical_ellipsis_menu_controller"; +import vertical_ellipsis_menu_controller from "../../../app/components/vertical_ellipsis_menu/component_controller"; describe("VerticalEllipsisMenuController test", () => { beforeAll(() => { From 2013750732dbdf1936214733319a05f0b55d8ff7 Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 30 Nov 2023 12:09:57 +1100 Subject: [PATCH 3/4] Rename files to match recommended naming scheme This naming scheme removes some duplication which is nice, but it's a little strange and results in a longer name overall. I don't like it very much because: - filenames don't include the component's actual name. This makes it slightly harder to find them in my text editor (but I'd probably get used to that) - the namespace and class naming isn't exactly right. This is _the_ vertical_ellipsis_menu, not a subcomponent. - the stimulus controller name is now longer, adding more cruft to the HTML. Lots of discussion here: https://github.com/ViewComponent/view_component/discussions/67 People tried to come up with a better way (and I was tempted to try myself). It seems this approach won. I guess it's not so bad if your component names are shorter. --- .../component.html.haml} | 0 app/components/vertical_ellipsis_menu/component.rb | 4 ++++ .../component.scss} | 0 .../component_controller.js | 0 app/components/vertical_ellipsis_menu_component.rb | 4 ---- app/views/admin/products_v3/_table.html.haml | 4 ++-- app/webpacker/css/admin_v3/all.scss | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) rename app/components/{vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.html.haml => vertical_ellipsis_menu/component.html.haml} (100%) create mode 100644 app/components/vertical_ellipsis_menu/component.rb rename app/components/{vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.scss => vertical_ellipsis_menu/component.scss} (100%) rename app/components/{vertical_ellipsis_menu_component => vertical_ellipsis_menu}/component_controller.js (100%) delete mode 100644 app/components/vertical_ellipsis_menu_component.rb diff --git a/app/components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.html.haml b/app/components/vertical_ellipsis_menu/component.html.haml similarity index 100% rename from app/components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.html.haml rename to app/components/vertical_ellipsis_menu/component.html.haml diff --git a/app/components/vertical_ellipsis_menu/component.rb b/app/components/vertical_ellipsis_menu/component.rb new file mode 100644 index 0000000000..6c864e1ac7 --- /dev/null +++ b/app/components/vertical_ellipsis_menu/component.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class VerticalEllipsisMenu::Component < ViewComponent::Base +end diff --git a/app/components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.scss b/app/components/vertical_ellipsis_menu/component.scss similarity index 100% rename from app/components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.scss rename to app/components/vertical_ellipsis_menu/component.scss diff --git a/app/components/vertical_ellipsis_menu_component/component_controller.js b/app/components/vertical_ellipsis_menu/component_controller.js similarity index 100% rename from app/components/vertical_ellipsis_menu_component/component_controller.js rename to app/components/vertical_ellipsis_menu/component_controller.js diff --git a/app/components/vertical_ellipsis_menu_component.rb b/app/components/vertical_ellipsis_menu_component.rb deleted file mode 100644 index 4915eaafb5..0000000000 --- a/app/components/vertical_ellipsis_menu_component.rb +++ /dev/null @@ -1,4 +0,0 @@ -# frozen_string_literal: true - -class VerticalEllipsisMenuComponent < ViewComponent::Base -end diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index 18009160d6..f7a4b69df6 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -67,7 +67,7 @@ %td.align-left .content= product.inherits_properties ? 'YES' : 'NO' #TODO: consider using https://github.com/RST-J/human_attribute_values, else use I18n.t (also below) %td.align-right - = render(VerticalEllipsisMenuComponent.new) do + = render(VerticalEllipsisMenu::Component.new) do = link_to t('admin.products_page.actions.edit'), edit_admin_product_path(product) = link_to t('admin.products_page.actions.clone'), clone_admin_product_path(product) @@ -97,5 +97,5 @@ %td.align-left -# empty %td.align-right - = render(VerticalEllipsisMenuComponent.new) do + = render(VerticalEllipsisMenu::Component.new) do = link_to t('admin.products_page.actions.edit'), edit_admin_product_variant_path(product, variant) diff --git a/app/webpacker/css/admin_v3/all.scss b/app/webpacker/css/admin_v3/all.scss index ed03376010..3ee85908f9 100644 --- a/app/webpacker/css/admin_v3/all.scss +++ b/app/webpacker/css/admin_v3/all.scss @@ -129,5 +129,5 @@ @import "app/components/help_modal_component/help_modal_component"; @import "app/components/confirm_modal_component/confirm_modal_component"; -@import "app/components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_component"; // admin_v3 and only V3 +@import "app/components/vertical_ellipsis_menu/component"; // admin_v3 and only V3 @import "app/webpacker/css/admin/trix.scss"; From 58215727bce4ed8ebd27c14324eba662f98dc88e Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 30 Nov 2023 13:46:39 +1100 Subject: [PATCH 4/4] [fixup] - Style/ClassAndModuleChildren: Use nested module/class definitions instead of compact style. - fix spec --- app/components/vertical_ellipsis_menu/component.rb | 4 +++- spec/components/vertical_ellipsis_menu_component_spec.rb | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/components/vertical_ellipsis_menu/component.rb b/app/components/vertical_ellipsis_menu/component.rb index 6c864e1ac7..696f0a0ad0 100644 --- a/app/components/vertical_ellipsis_menu/component.rb +++ b/app/components/vertical_ellipsis_menu/component.rb @@ -1,4 +1,6 @@ # frozen_string_literal: true -class VerticalEllipsisMenu::Component < ViewComponent::Base +module VerticalEllipsisMenu + class Component < ViewComponent::Base + end end diff --git a/spec/components/vertical_ellipsis_menu_component_spec.rb b/spec/components/vertical_ellipsis_menu_component_spec.rb index f073cd078e..cdbdea159f 100644 --- a/spec/components/vertical_ellipsis_menu_component_spec.rb +++ b/spec/components/vertical_ellipsis_menu_component_spec.rb @@ -2,10 +2,10 @@ require "spec_helper" -describe VerticalEllipsisMenuComponent, type: :component do +describe VerticalEllipsisMenu::Component, type: :component do it "displays the included links" do content = "Edit" - render_inline(VerticalEllipsisMenuComponent.new.with_content(content.html_safe)) + render_inline(described_class.new.with_content(content.html_safe)) expect(page).to have_selector "a", text: "Edit" end