From 8c6d3a27ec2304475e2eee8fd96f8005dd14a178 Mon Sep 17 00:00:00 2001 From: David Cook Date: Tue, 6 Feb 2024 14:34:12 +1100 Subject: [PATCH 1/7] Move modal action styles to parent component They were already being shared with a HelpModalComponent in fact. --- .../confirm_modal_component.scss | 22 ------------------ .../modal_component/modal_component.scss | 23 +++++++++++++++++++ app/webpacker/css/admin/all.scss | 1 - app/webpacker/css/admin_v3/all.scss | 1 - app/webpacker/css/darkswarm/all.scss | 1 - 5 files changed, 23 insertions(+), 25 deletions(-) delete mode 100644 app/components/confirm_modal_component/confirm_modal_component.scss diff --git a/app/components/confirm_modal_component/confirm_modal_component.scss b/app/components/confirm_modal_component/confirm_modal_component.scss deleted file mode 100644 index 2c54031f32..0000000000 --- a/app/components/confirm_modal_component/confirm_modal_component.scss +++ /dev/null @@ -1,22 +0,0 @@ -.modal-actions { - display: flex; - - &.justify-space-around { - justify-content: space-around; - } - - &.justify-end { - justify-content: flex-end; - input[type="button"] { - margin: 0 5px; - } - - @media only screen and (max-width: 1024px) { - flex-direction: column; - justify-content: space-around; - input[type="button"] { - margin: 5px 0; - } - } - } -} diff --git a/app/components/modal_component/modal_component.scss b/app/components/modal_component/modal_component.scss index 5e9f6ad8bd..13e105b73f 100644 --- a/app/components/modal_component/modal_component.scss +++ b/app/components/modal_component/modal_component.scss @@ -22,3 +22,26 @@ body.modal-open #admin-menu li.selected a::after { z-index: 0; } + +.modal-actions { + display: flex; + + &.justify-space-around { + justify-content: space-around; + } + + &.justify-end { + justify-content: flex-end; + input[type="button"] { + margin: 0 5px; + } + + @media only screen and (max-width: 1024px) { + flex-direction: column; + justify-content: space-around; + input[type="button"] { + margin: 5px 0; + } + } + } +} diff --git a/app/webpacker/css/admin/all.scss b/app/webpacker/css/admin/all.scss index 2cb175b8f7..d9ad7c0008 100644 --- a/app/webpacker/css/admin/all.scss +++ b/app/webpacker/css/admin/all.scss @@ -125,5 +125,4 @@ @import "components/tom_select"; @import "app/components/modal_component/modal_component"; -@import "app/components/confirm_modal_component/confirm_modal_component"; @import "app/webpacker/css/admin/trix.scss"; diff --git a/app/webpacker/css/admin_v3/all.scss b/app/webpacker/css/admin_v3/all.scss index 274e76a76e..7ee27d28ac 100644 --- a/app/webpacker/css/admin_v3/all.scss +++ b/app/webpacker/css/admin_v3/all.scss @@ -129,7 +129,6 @@ @import "components/tom_select"; // admin_v3 @import "app/components/modal_component/modal_component"; -@import "app/components/confirm_modal_component/confirm_modal_component"; @import "app/components/vertical_ellipsis_menu/component"; // admin_v3 and only V3 @import "app/webpacker/css/admin/trix.scss"; diff --git a/app/webpacker/css/darkswarm/all.scss b/app/webpacker/css/darkswarm/all.scss index be25b548d5..cbab208f24 100644 --- a/app/webpacker/css/darkswarm/all.scss +++ b/app/webpacker/css/darkswarm/all.scss @@ -78,4 +78,3 @@ ofn-modal { @import '../admin/shared/scroll_bar'; @import 'app/components/modal_component/modal_component'; -@import 'app/components/confirm_modal_component/confirm_modal_component'; From 784f74f46683c1df9e919213638331f2b662810b Mon Sep 17 00:00:00 2001 From: David Cook Date: Tue, 6 Feb 2024 14:51:13 +1100 Subject: [PATCH 2/7] Align buttons to the right --- app/views/admin/products_v3/_edit_image.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/admin/products_v3/_edit_image.html.haml b/app/views/admin/products_v3/_edit_image.html.haml index b4f45ace5d..5ef826c268 100644 --- a/app/views/admin/products_v3/_edit_image.html.haml +++ b/app/views/admin/products_v3/_edit_image.html.haml @@ -9,7 +9,7 @@ %input{ type: :hidden, name: :return_url, value: return_url} = f.hidden_field :viewable_id, value: product.id - %p + .modal-actions.justify-end %input{ class: "secondary", type: 'button', value: t('.close'), "data-action": "click->modal#close" } -# label.button provides a handy shortcut to open the file selector on click. Unfortunately this trick isn't keyboard accessible though.. = f.label :attachment, t(".upload"), class: "button primary icon-upload-alt" From 6189feadaacd157b95f2cd69396aec248e3859a8 Mon Sep 17 00:00:00 2001 From: David Cook Date: Tue, 6 Feb 2024 14:52:44 +1100 Subject: [PATCH 3/7] Prevent modal covered by scrollbar Using units relative to the document, rather than the screen. I don't think that remaining min-height should be there at all, but reveal-modal is used a lot so I don't know for sure. --- app/webpacker/css/admin/modals.scss | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/webpacker/css/admin/modals.scss b/app/webpacker/css/admin/modals.scss index b67c6e461a..72dc9dff88 100644 --- a/app/webpacker/css/admin/modals.scss +++ b/app/webpacker/css/admin/modals.scss @@ -16,7 +16,7 @@ dialog { display: none; position: absolute; z-index: 1005; - width: 100vw; + width: 100%; top: 0; border-radius: 0.4em; border: 0px none; @@ -75,8 +75,6 @@ dialog.full { top: 0; left: 0; height: 100%; - height: 100vh; - min-height: 100vh; max-width: none !important; margin-left: 0 !important; } @@ -161,7 +159,7 @@ dialog[open] { @media only screen and (max-width: 40em) { .reveal-modal, dialog { - min-height: 100vh; + min-height: 100%; } } @@ -212,7 +210,7 @@ dialog[open] { } .reveal-modal.full, dialog.full { - width: 100vw; + width: 100%; max-width: 62.5rem; left: 0; right: 0; From b3692d74682f5ee1763d04b1ca04591d024c908c Mon Sep 17 00:00:00 2001 From: David Cook Date: Tue, 6 Feb 2024 15:00:29 +1100 Subject: [PATCH 4/7] Remove min-height for ModalComponent --- app/components/modal_component/modal_component.scss | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/components/modal_component/modal_component.scss b/app/components/modal_component/modal_component.scss index 13e105b73f..ec878e62ea 100644 --- a/app/components/modal_component/modal_component.scss +++ b/app/components/modal_component/modal_component.scss @@ -3,6 +3,8 @@ visibility: visible; position: fixed; top: 3em; + min-height: auto; // reset from reveal-modal + &.in { padding: 1.2rem; } From d24348d0f26460cffde4909991ea74288e851ea2 Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 7 Feb 2024 10:13:17 +1100 Subject: [PATCH 5/7] Increase button size And fix alignment for smaller screens. --- app/components/modal_component/modal_component.scss | 1 + app/views/admin/products_v3/_edit_image.html.haml | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/components/modal_component/modal_component.scss b/app/components/modal_component/modal_component.scss index ec878e62ea..d8c3cdd3fc 100644 --- a/app/components/modal_component/modal_component.scss +++ b/app/components/modal_component/modal_component.scss @@ -27,6 +27,7 @@ body.modal-open #admin-menu li.selected a::after { .modal-actions { display: flex; + text-align: center; // Ensure text inside fullwidth buttons are centred on small screens &.justify-space-around { justify-content: space-around; diff --git a/app/views/admin/products_v3/_edit_image.html.haml b/app/views/admin/products_v3/_edit_image.html.haml index 5ef826c268..218bd29ef5 100644 --- a/app/views/admin/products_v3/_edit_image.html.haml +++ b/app/views/admin/products_v3/_edit_image.html.haml @@ -10,7 +10,7 @@ = f.hidden_field :viewable_id, value: product.id .modal-actions.justify-end - %input{ class: "secondary", type: 'button', value: t('.close'), "data-action": "click->modal#close" } + %input{ class: "secondary relaxed", type: 'button', value: t('.close'), "data-action": "click->modal#close" } -# label.button provides a handy shortcut to open the file selector on click. Unfortunately this trick isn't keyboard accessible though.. - = f.label :attachment, t(".upload"), class: "button primary icon-upload-alt" + = f.label :attachment, t(".upload"), class: "button primary relaxed icon-upload-alt" = f.file_field :attachment, accept: "image/*", style: "display: none", "data-action": "change->form#submit" From 98cfc68c3a44bd9836ba7a28144994ef1e7f8ec6 Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 7 Feb 2024 12:28:43 +1100 Subject: [PATCH 6/7] Add option for modal class --- app/components/modal_component.rb | 3 ++- app/components/modal_component/modal_component.html.haml | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/components/modal_component.rb b/app/components/modal_component.rb index a538eb19b1..1de8852a8c 100644 --- a/app/components/modal_component.rb +++ b/app/components/modal_component.rb @@ -1,10 +1,11 @@ # frozen_string_literal: true class ModalComponent < ViewComponent::Base - def initialize(id:, close_button: true, instant: false) + def initialize(id:, close_button: true, instant: false, modal_class: :small) @id = id @close_button = close_button @instant = instant + @modal_class = modal_class end private diff --git a/app/components/modal_component/modal_component.html.haml b/app/components/modal_component/modal_component.html.haml index 1677d71b3f..7be73deca4 100644 --- a/app/components/modal_component/modal_component.html.haml +++ b/app/components/modal_component/modal_component.html.haml @@ -1,6 +1,6 @@ %div{ id: @id, "data-controller": "modal", "data-action": "keyup@document->modal#closeIfEscapeKey", "data-modal-instant-value": @instant } .reveal-modal-bg.fade{ "data-modal-target": "background", "data-action": "click->modal#close" } - .reveal-modal.fade.small.modal-component{ "data-modal-target": "modal" } + .reveal-modal.fade.modal-component{ "data-modal-target": "modal", class: @modal_class } = content - if close_button? From 5e4dd3864f8c44e27d7389cfae9044bba1d04f66 Mon Sep 17 00:00:00 2001 From: David Cook Date: Wed, 7 Feb 2024 12:35:23 +1100 Subject: [PATCH 7/7] Update image and modal size Using a new 'fit' modal size. On smaller screens, we need to allow the image to shrink. That's a good general rule, but I was hesitant to make it a global rule.. --- app/components/modal_component/modal_component.scss | 6 ++++++ app/views/admin/products_v3/_edit_image.html.haml | 5 +++-- app/webpacker/css/admin/modals.scss | 4 ++++ spec/system/admin/products_v3/products_spec.rb | 6 +++--- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/app/components/modal_component/modal_component.scss b/app/components/modal_component/modal_component.scss index d8c3cdd3fc..72c0e74088 100644 --- a/app/components/modal_component/modal_component.scss +++ b/app/components/modal_component/modal_component.scss @@ -18,6 +18,12 @@ p { margin-bottom: 0.5em; } + + img { + // Ensure image fits in container + max-width: 100%; + height: auto; + } } /* prevent arrow on selected admin menu item appearing above modal */ diff --git a/app/views/admin/products_v3/_edit_image.html.haml b/app/views/admin/products_v3/_edit_image.html.haml index 218bd29ef5..57fccaac76 100644 --- a/app/views/admin/products_v3/_edit_image.html.haml +++ b/app/views/admin/products_v3/_edit_image.html.haml @@ -1,7 +1,8 @@ -= render ModalComponent.new id: "#modal_edit_product_image_#{image.id}", instant: true, close_button: false do += render ModalComponent.new id: "#modal_edit_product_image_#{image.id}", instant: true, close_button: false, modal_class: :fit do %h2= t(".title") - %p= image_tag image.persisted? ? image.url(:product) : Spree::Image.default_image_url(:product) + -# Display image in the same way it appears in the shopfront popup + %p= image_tag image.persisted? ? image.url(:large) : Spree::Image.default_image_url(:large), width: 433, height: 433 -# Submit to controller, because StimulusReflex doesn't support file uploads = form_for [:admin, product, image], diff --git a/app/webpacker/css/admin/modals.scss b/app/webpacker/css/admin/modals.scss index 72dc9dff88..20c27e9a67 100644 --- a/app/webpacker/css/admin/modals.scss +++ b/app/webpacker/css/admin/modals.scss @@ -168,6 +168,10 @@ dialog[open] { dialog { top: 6.25rem; } + .reveal-modal.fit, + dialog.fit { + width: fit-content; + } .reveal-modal.tiny, dialog.tiny { width: 30%; diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index 642265abea..52bc19c387 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -343,7 +343,7 @@ describe 'As an admin, I can manage products', feature: :admin_style_v3 do end expect(page).to have_content /Image has been successfully (updated|created)/ - expect(product.image.reload.url(:product)).to match /500.jpg$/ + expect(product.image.reload.url(:large)).to match /500.jpg$/ within row_containing_name("Apples") do expect_page_to_have_image('500.jpg') @@ -353,14 +353,14 @@ describe 'As an admin, I can manage products', feature: :admin_style_v3 do context "with existing image" do let!(:product) { create(:product_with_image, name: "Apples") } - let(:current_img_url) { product.image.url(:product) } + let(:current_img_url) { product.image.url(:large) } include_examples "updating image" end context "with default image" do let!(:product) { create(:product, name: "Apples") } - let(:current_img_url) { Spree::Image.default_image_url(:product) } + let(:current_img_url) { Spree::Image.default_image_url(:large) } include_examples "updating image" end