From c6fc139f6217b3043fc1f6aefd6cfc6205186a86 Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 23 Nov 2023 17:04:09 +1100 Subject: [PATCH 1/5] Always generate components in subfolders Currently all our view components are in subfolders, so we probably want to keep it that way. --- config/application.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/application.rb b/config/application.rb index 52c77186c4..116fb5970a 100644 --- a/config/application.rb +++ b/config/application.rb @@ -248,5 +248,7 @@ module Openfoodnetwork config.active_storage.url_options = config.action_controller.default_url_options config.exceptions_app = self.routes + + config.view_component.generate.sidecar = true # Always generate components in subfolders end end From 2bd2bea7b7113c67e3917b71003fb3183ce89f6d Mon Sep 17 00:00:00 2001 From: David Cook Date: Fri, 24 Nov 2023 16:14:28 +1100 Subject: [PATCH 2/5] Remove need for class Generally, I would say that style rules should have a BEM-style class name, but in this case it's terribly convenient to support a tags directly. --- .../admin/products_v3/components/_product_actions.html.haml | 6 +++--- .../css/admin_v3/components/vertical_ellipsis_menu.scss | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/admin/products_v3/components/_product_actions.html.haml b/app/views/admin/products_v3/components/_product_actions.html.haml index 8152ec1584..cd50dd497b 100644 --- a/app/views/admin/products_v3/components/_product_actions.html.haml +++ b/app/views/admin/products_v3/components/_product_actions.html.haml @@ -2,7 +2,7 @@ %i.fa.fa-ellipsis-v{ "data-action": "click->vertical-ellipsis-menu#toggle" } .vertical-ellipsis-menu-content{ "data-vertical-ellipsis-menu-target": "content" } - if defined?(variant) - = link_to t('admin.products_page.actions.edit'), edit_admin_product_variant_path(product, variant), class: "vertical-ellipsis-menu-content-item" + = link_to t('admin.products_page.actions.edit'), edit_admin_product_variant_path(product, variant) - else - = link_to t('admin.products_page.actions.edit'), edit_admin_product_path(product), class: "vertical-ellipsis-menu-content-item" - = link_to t('admin.products_page.actions.clone'), clone_admin_product_path(product), class: "vertical-ellipsis-menu-content-item" + = 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) diff --git a/app/webpacker/css/admin_v3/components/vertical_ellipsis_menu.scss b/app/webpacker/css/admin_v3/components/vertical_ellipsis_menu.scss index de35c98455..9434a94bc4 100644 --- a/app/webpacker/css/admin_v3/components/vertical_ellipsis_menu.scss +++ b/app/webpacker/css/admin_v3/components/vertical_ellipsis_menu.scss @@ -30,7 +30,7 @@ display: block; } - .vertical-ellipsis-menu-content-item { + & > a { display: block; padding: 5px 10px; cursor: pointer; From 968a224da60dcf0ef5b3f7919c8e9727edf31828 Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 23 Nov 2023 17:29:14 +1100 Subject: [PATCH 3/5] Move vertical ellipsis menu to a ViewComponent Javascript hasn't been moved in, as we don't seem to be set up for that yet. We could make it smarter, and pass in an array of parameters to build the links (as in _order_links.html.haml). But why make it complicated if we don't need to? --- app/components/vertical_ellipsis_menu_component.rb | 4 ++++ .../vertical_ellipsis_menu_component.html.haml | 4 ++++ .../vertical_ellipsis_menu_component.scss} | 0 .../components/_product_actions.html.haml | 14 ++++++-------- app/webpacker/css/admin_v3/all.scss | 2 +- .../vertical_ellipsis_menu_component_spec.rb | 12 ++++++++++++ 6 files changed, 27 insertions(+), 9 deletions(-) create mode 100644 app/components/vertical_ellipsis_menu_component.rb create mode 100644 app/components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.html.haml rename app/{webpacker/css/admin_v3/components/vertical_ellipsis_menu.scss => components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.scss} (100%) create mode 100644 spec/components/vertical_ellipsis_menu_component_spec.rb diff --git a/app/components/vertical_ellipsis_menu_component.rb b/app/components/vertical_ellipsis_menu_component.rb new file mode 100644 index 0000000000..4915eaafb5 --- /dev/null +++ b/app/components/vertical_ellipsis_menu_component.rb @@ -0,0 +1,4 @@ +# frozen_string_literal: true + +class VerticalEllipsisMenuComponent < ViewComponent::Base +end 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 new file mode 100644 index 0000000000..ea67c8ef00 --- /dev/null +++ b/app/components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.html.haml @@ -0,0 +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" } + = content diff --git a/app/webpacker/css/admin_v3/components/vertical_ellipsis_menu.scss b/app/components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.scss similarity index 100% rename from app/webpacker/css/admin_v3/components/vertical_ellipsis_menu.scss rename to app/components/vertical_ellipsis_menu_component/vertical_ellipsis_menu_component.scss diff --git a/app/views/admin/products_v3/components/_product_actions.html.haml b/app/views/admin/products_v3/components/_product_actions.html.haml index cd50dd497b..a37221695f 100644 --- a/app/views/admin/products_v3/components/_product_actions.html.haml +++ b/app/views/admin/products_v3/components/_product_actions.html.haml @@ -1,8 +1,6 @@ -.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" } - - if defined?(variant) - = link_to t('admin.products_page.actions.edit'), edit_admin_product_variant_path(product, variant) - - else - = 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) += render(VerticalEllipsisMenuComponent.new) do + - if defined?(variant) + = link_to t('admin.products_page.actions.edit'), edit_admin_product_variant_path(product, variant) + - else + = 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) diff --git a/app/webpacker/css/admin_v3/all.scss b/app/webpacker/css/admin_v3/all.scss index f2d6fe5bd4..ed03376010 100644 --- a/app/webpacker/css/admin_v3/all.scss +++ b/app/webpacker/css/admin_v3/all.scss @@ -113,7 +113,6 @@ @import "../admin/reports"; @import "components/select2"; // admin_v3 @import "components/sidebar-item"; // admin_v3 -@import "components/vertical_ellipsis_menu"; // admin_v3 and only V3 @import "../admin/side_menu"; @import "../admin/tables"; @import "../admin/tag_rules"; @@ -130,4 +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/webpacker/css/admin/trix.scss"; diff --git a/spec/components/vertical_ellipsis_menu_component_spec.rb b/spec/components/vertical_ellipsis_menu_component_spec.rb new file mode 100644 index 0000000000..f073cd078e --- /dev/null +++ b/spec/components/vertical_ellipsis_menu_component_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require "spec_helper" + +describe VerticalEllipsisMenuComponent, type: :component do + it "displays the included links" do + content = "Edit" + render_inline(VerticalEllipsisMenuComponent.new.with_content(content.html_safe)) + + expect(page).to have_selector "a", text: "Edit" + end +end From d989b8ad2fde49280f69fe22b7da519a2ad579d0 Mon Sep 17 00:00:00 2001 From: David Cook Date: Fri, 24 Nov 2023 16:28:00 +1100 Subject: [PATCH 4/5] Remove unnecessary partial Yay, now it's compact enough to fit in the table partial. (Although we should probably try to reduce that one down one day..) --- app/views/admin/products_v3/_table.html.haml | 8 ++++++-- .../products_v3/components/_product_actions.html.haml | 6 ------ 2 files changed, 6 insertions(+), 8 deletions(-) delete mode 100644 app/views/admin/products_v3/components/_product_actions.html.haml diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index 3c6d40fa74..18009160d6 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -67,7 +67,10 @@ %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 partial: 'admin/products_v3/components/product_actions', locals: { product: product } + = render(VerticalEllipsisMenuComponent.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) + - product.variants.each do |variant| = form.fields_for("products][][variants_attributes][", variant, index: nil) do |variant_form| %tr.condensed @@ -94,4 +97,5 @@ %td.align-left -# empty %td.align-right - = render partial: 'admin/products_v3/components/product_actions', locals: { product: product, variant: variant } + = render(VerticalEllipsisMenuComponent.new) do + = link_to t('admin.products_page.actions.edit'), edit_admin_product_variant_path(product, variant) diff --git a/app/views/admin/products_v3/components/_product_actions.html.haml b/app/views/admin/products_v3/components/_product_actions.html.haml deleted file mode 100644 index a37221695f..0000000000 --- a/app/views/admin/products_v3/components/_product_actions.html.haml +++ /dev/null @@ -1,6 +0,0 @@ -= render(VerticalEllipsisMenuComponent.new) do - - if defined?(variant) - = link_to t('admin.products_page.actions.edit'), edit_admin_product_variant_path(product, variant) - - else - = 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) From e53960bae7a01ca384b93f3fe46ce34696e3178f Mon Sep 17 00:00:00 2001 From: David Cook Date: Fri, 24 Nov 2023 16:35:45 +1100 Subject: [PATCH 5/5] Disable Rails/OutputSafety for specs This cop is to protect against user input. There's no user input (or users) in specs. --- .rubocop_styleguide.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.rubocop_styleguide.yml b/.rubocop_styleguide.yml index 5459d63ebc..5606130daa 100644 --- a/.rubocop_styleguide.yml +++ b/.rubocop_styleguide.yml @@ -70,6 +70,10 @@ Rails/SkipsModelValidations: - "update_column" - "update_columns" +Rails/OutputSafety: + Exclude: + - 'spec/**/*' + Style/Documentation: Enabled: false