diff --git a/app/assets/javascripts/admin/spree/base.js.erb b/app/assets/javascripts/admin/spree/base.js.erb index 8a2015d786..99aada8cb5 100644 --- a/app/assets/javascripts/admin/spree/base.js.erb +++ b/app/assets/javascripts/admin/spree/base.js.erb @@ -32,9 +32,6 @@ jQuery(function($) { }); } - // Make flash messages dissapear - setTimeout('$(".flash").fadeOut()', 5000); - // Highlight hovered table column $('table tbody tr td.actions a').hover(function(){ var tr = $(this).closest('tr'); diff --git a/app/components/vertical_ellipsis_menu/component.scss b/app/components/vertical_ellipsis_menu/component.scss index 9434a94bc4..03b5cb39d8 100644 --- a/app/components/vertical_ellipsis_menu/component.scss +++ b/app/components/vertical_ellipsis_menu/component.scss @@ -20,7 +20,7 @@ padding-top: 5px; padding-bottom: 5px; background-color: white; - @include defaultBoxShadow; + box-shadow: $box-shadow; border-radius: 3px; min-width: 80px; display: none; diff --git a/app/controllers/spree/admin/products_controller.rb b/app/controllers/spree/admin/products_controller.rb index 3ecc96dab6..e153faff38 100644 --- a/app/controllers/spree/admin/products_controller.rb +++ b/app/controllers/spree/admin/products_controller.rb @@ -82,8 +82,9 @@ module Spree def clone @new = @product.duplicate - @new.save + raise "Clone failed" unless @new.save + flash[:success] = t('.success') redirect_to spree.admin_products_url end diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index c75eddce24..4154a7b61f 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -38,13 +38,12 @@ class ProductsReflex < ApplicationReflex @products = product_set.collection # use instance variable mainly for testing if product_set.save - # flash[:success] = with_locale { I18n.t('.success') } - # morph_admin_flashes # ERROR: selector morph type has already been set + flash[:success] = I18n.t('admin.products_v3.bulk_update.success') elsif product_set.errors.present? @error_counts = { saved: product_set.saved_count, invalid: product_set.invalid.count } end - render_products_form + render_products_form_with_flash end private @@ -85,9 +84,10 @@ class ProductsReflex < ApplicationReflex morph :nothing end - def render_products_form + def render_products_form_with_flash locals = { products: @products } locals[:error_counts] = @error_counts if @error_counts.present? + locals[:flashes] = flash if flash.any? cable_ready.replace( selector: "#products-form", diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index 519738169c..0462529857 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -4,6 +4,7 @@ 'data-controller': "bulk-form", 'data-bulk-form-disable-selector-value': "#sort,#filters", 'data-bulk-form-error-value': defined?(error_counts), } do |form| + = render(partial: "admin/shared/flashes", locals: { flashes: }) if defined? flashes %fieldset.form-actions{ class: ("hidden" unless defined?(error_counts)), 'data-bulk-form-target': "actions" } .container .status.eleven.columns diff --git a/app/views/admin/products_v3/index.html.haml b/app/views/admin/products_v3/index.html.haml index 581dcc2605..82a4e5f38e 100644 --- a/app/views/admin/products_v3/index.html.haml +++ b/app/views/admin/products_v3/index.html.haml @@ -11,7 +11,9 @@ = render partial: 'spree/admin/shared/product_sub_menu' #products_v3_page{ "data-controller": "products" } - #loading-spinner.spinner-container{ "data-controller": "loading", "data-products-target": "loading" } - .spinner - = t('.loading') + + .spinner-overlay{ "data-controller": "loading", "data-products-target": "loading" } + .spinner-container + .spinner + = t('.loading') #products-content diff --git a/app/views/admin/shared/_flashes.html.haml b/app/views/admin/shared/_flashes.html.haml index 57673b3f48..4e8b94dace 100644 --- a/app/views/admin/shared/_flashes.html.haml +++ b/app/views/admin/shared/_flashes.html.haml @@ -1,6 +1,8 @@ -#flashes +.flash-container - if defined? flashes - flashes.each do |type, msg| - .animate-show{"data-controller": "flash"} + .animate-show{"data-controller": "flash", "data-flash-auto-close-value": type == 'success'} .flash{type: "#{type}", class: "#{type}"} - %span= msg + .msg= msg + .actions + %button.secondary{"data-action": "click->flash#close"}= t('.dismiss') diff --git a/app/views/spree/layouts/_admin_body.html.haml b/app/views/spree/layouts/_admin_body.html.haml index 4ca442a850..76f1f77e26 100644 --- a/app/views/spree/layouts/_admin_body.html.haml +++ b/app/views/spree/layouts/_admin_body.html.haml @@ -3,7 +3,7 @@ = yield :stripe_js #wrapper - .flash-container + #flashes = render partial: "admin/shared/flashes", locals: { flashes: flash } = render partial: "spree/layouts/admin/progress_spinner" diff --git a/app/webpacker/controllers/flash_controller.js b/app/webpacker/controllers/flash_controller.js index c30c08bcab..84dfe7eaf9 100644 --- a/app/webpacker/controllers/flash_controller.js +++ b/app/webpacker/controllers/flash_controller.js @@ -5,7 +5,25 @@ document.addEventListener("turbolinks:before-cache", () => ); export default class extends Controller { - close() { + static values = { + autoClose: Boolean, + }; + + connect() { + if (this.autoCloseValue) { + setTimeout(this.close.bind(this), 5000); + } + } + + close(e) { + // Fade out + this.element.classList.remove("animate-show"); + this.element.classList.add("animate-hide-500"); + setTimeout(this.remove.bind(this), 500); + e && e.preventDefault(); // Prevent submitting if we're inside a form + } + + remove() { this.element.remove(); } } diff --git a/app/webpacker/css/admin/animations.scss b/app/webpacker/css/admin/animations.scss index 7b6e451078..6dffab1cfe 100644 --- a/app/webpacker/css/admin/animations.scss +++ b/app/webpacker/css/admin/animations.scss @@ -11,13 +11,23 @@ } @keyframes fade-out-hide { - 0% {opacity: 1; visibility: visible;} - 99% {opacity: 0; visibility: visible;} - 100% {opacity: 0; visibility: hidden;} + 0% { + opacity: 1; + visibility: visible; + } + 99% { + opacity: 0; + visibility: visible; + } + 100% { + opacity: 0; + visibility: hidden; + } } .animate-hide-500 { animation: fade-out-hide 0.5s; + opacity: 0; // Stay invisible after animation } // @-webkit-keyframes slideOutDown diff --git a/app/webpacker/css/admin/components/messages.scss b/app/webpacker/css/admin/components/messages.scss index 61dd751a55..f92498a599 100644 --- a/app/webpacker/css/admin/components/messages.scss +++ b/app/webpacker/css/admin/components/messages.scss @@ -57,6 +57,10 @@ &:nth-child(3) { padding: 20px; } + + .actions { + display: none; /* avoid adding new button on old design */ + } } } diff --git a/app/webpacker/css/admin_v3/all.scss b/app/webpacker/css/admin_v3/all.scss index 3ee85908f9..6e4fe6b7bb 100644 --- a/app/webpacker/css/admin_v3/all.scss +++ b/app/webpacker/css/admin_v3/all.scss @@ -56,7 +56,7 @@ @import "../admin/components/dialogs"; @import "../admin/components/input"; @import "../admin/components/jquery_dialog"; -@import "../admin/components/messages"; +@import "components/messages"; // admin_v3 @import "components/navigation"; // admin_v3 @import "../admin/components/ng-cloak"; @import "../admin/components/page_actions"; diff --git a/app/webpacker/css/admin_v3/components/buttons.scss b/app/webpacker/css/admin_v3/components/buttons.scss index 13ecab6d7b..a5ab9c3b7f 100644 --- a/app/webpacker/css/admin_v3/components/buttons.scss +++ b/app/webpacker/css/admin_v3/components/buttons.scss @@ -53,14 +53,14 @@ button:not(.plain):not(.trix-button), } &.secondary { - background-color: transparent; + background-color: $color-btn-secondary-bg; border: 1px solid $color-btn-bg; color: $color-btn-bg; &:hover { background-color: $color-11; - border: 1px solid $color-btn-hover-bg; - color: $color-btn-hover-bg; + border: 1px solid $color-btn-secondary-hover-bg; + color: $color-btn-secondary-hover-bg; } &:active, diff --git a/app/webpacker/css/admin_v3/components/messages.scss b/app/webpacker/css/admin_v3/components/messages.scss new file mode 100644 index 0000000000..1d574a9a07 --- /dev/null +++ b/app/webpacker/css/admin_v3/components/messages.scss @@ -0,0 +1,80 @@ +.errorExplanation { + padding: 10px; + border: 1px solid very-light($color-error, 12); + background-color: very-light($color-error, 6); + border-radius: 3px; + color: $color-error; + margin-bottom: 15px; + + h2 { + font-size: 140%; + color: $color-error; + margin-bottom: 5px; + } + + p { + padding: 10px 0; + } + + ul { + list-style-position: inside; + + li { + font-weight: $font-weight-bold; + } + } +} + +.flash-container { + position: fixed; + left: 0; + bottom: 0; + width: 100%; + z-index: 1000; + display: flex; + justify-content: center; + + .flash { + position: relative; + display: flex; + align-items: center; + bottom: 3.5rem; + padding: 0.25rem; + min-width: 24rem; + max-width: 48.25em; + font-weight: 600; + background-color: $light-grey; + border-radius: $border-radius; + box-shadow: $box-shadow; + + &.success { + color: $color-flash-success-text; + background-color: $color-flash-success-bg; + } + &.notice { + color: $color-flash-notice-text; + background-color: $color-flash-notice-bg; + border-left: $border-radius solid $red; + } + &.error { + color: $color-flash-error-text; + background-color: $color-flash-error-bg; + } + + .msg { + flex-grow: 1; + margin: 0.25rem 0.75rem; + } + } +} + +.notice:not(.flash) { + padding: 1rem; + margin-bottom: 1.5rem; + background-color: $spree-light-blue; + border-radius: $border-radius; + + a { + font-weight: bold; + } +} diff --git a/app/webpacker/css/admin_v3/components/navigation.scss b/app/webpacker/css/admin_v3/components/navigation.scss index ecefc92369..4ef5d5a06c 100644 --- a/app/webpacker/css/admin_v3/components/navigation.scss +++ b/app/webpacker/css/admin_v3/components/navigation.scss @@ -61,7 +61,7 @@ nav.menu { } #admin-menu { - @include defaultBoxShadow; + box-shadow: $box-shadow; li { .dropdown { diff --git a/app/webpacker/css/admin_v3/components/pagination.scss b/app/webpacker/css/admin_v3/components/pagination.scss index 82642f54a6..98df96f06e 100644 --- a/app/webpacker/css/admin_v3/components/pagination.scss +++ b/app/webpacker/css/admin_v3/components/pagination.scss @@ -20,7 +20,7 @@ display: inline-block; text-align: center; background-color: $color-1; - @include defaultBoxShadow; + box-shadow: $box-shadow; border-radius: 4px; border: none; color: $color-8; @@ -77,7 +77,7 @@ background-color: $white; color: $near-black; - box-shadow: $color-btn-shadow; + box-shadow: $box-shadow; &.active { color: $white; diff --git a/app/webpacker/css/admin_v3/components/spinner.scss b/app/webpacker/css/admin_v3/components/spinner.scss index d6694b3ed2..fd7235ffe3 100644 --- a/app/webpacker/css/admin_v3/components/spinner.scss +++ b/app/webpacker/css/admin_v3/components/spinner.scss @@ -1,16 +1,23 @@ -.spinner-container { +.spinner-overlay { position: absolute; - width: 100%; + left: 0; + right: 0; height: 100%; min-height: 200px; + background: rgba(255, 255, 255, 0.8); + z-index: 2; +} + +.spinner-container { + position: fixed; + left: 0; + right: 0; display: flex; - justify-content: flex-start; + justify-content: center; align-items: center; flex-direction: column; gap: 40px; font-size: 24px; - background: rgba(255, 255, 255, 0.8); - z-index: 2; &.hidden { display: none; diff --git a/app/webpacker/css/admin_v3/globals/variables.scss b/app/webpacker/css/admin_v3/globals/variables.scss index 7eecf26ea8..e41f2a9597 100644 --- a/app/webpacker/css/admin_v3/globals/variables.scss +++ b/app/webpacker/css/admin_v3/globals/variables.scss @@ -25,6 +25,12 @@ $color-success: $green !default; $color-notice: $yellow !default; $color-warning: $red !default; $color-error: $red !default; +$color-flash-success-text: $white !default; +$color-flash-success-bg: $near-black !default; +$color-flash-notice-text: $color-body-text !default; +$color-flash-notice-bg: $fair-pink !default; +$color-flash-error-text: $white !default; +$color-flash-error-bg: $color-error !default; // Table styles $color-tbl-bg: $light-grey !default; @@ -40,7 +46,6 @@ $padding-tbl-cell-relaxed: 12px 12px; // Button colors $color-btn-bg: $teal !default; $color-btn-text: $white !default; -$color-btn-shadow: 0px 1px 0px rgba(0, 0, 0, 0.05), 0px 2px 2px rgba(0, 0, 0, 0.07) !default; $color-btn-hover-bg: $orient !default; $color-btn-hover-text: $white !default; $color-btn-hover-border: $dark-blue !default; @@ -48,6 +53,8 @@ $color-btn-disabled-bg: $medium-grey !default; $color-btn-disabled-text: $lighter-grey !default; $color-btn-red-bg: $red !default; $color-btn-red-hover-bg: $roof-terracotta !default; +$color-btn-secondary-bg: $white !default; +$color-btn-secondary-hover-bg: $orient !default; // Actions colors $color-action-edit-bg: very-light($teal) !default; @@ -161,6 +168,7 @@ $h2-size: $h3-size + 2 !default; $h1-size: $h2-size + 2 !default; $border-radius: 4px !default; +$box-shadow: 0px 1px 0px rgba(0, 0, 0, 0.05), 0px 2px 2px rgba(0, 0, 0, 0.07) !default; $font-weight-bold: 600 !default; $font-weight-normal: 400 !default; diff --git a/app/webpacker/css/admin_v3/mixins.scss b/app/webpacker/css/admin_v3/mixins.scss index 5301558d32..c2471a52ba 100644 --- a/app/webpacker/css/admin_v3/mixins.scss +++ b/app/webpacker/css/admin_v3/mixins.scss @@ -1,7 +1,3 @@ -@mixin defaultBoxShadow { - box-shadow: 0px 1px 0px rgba(0, 0, 0, 0.05), 0px 2px 2px rgba(0, 0, 0, 0.07); -} - @mixin arrowDown { content: " "; display: block; diff --git a/app/webpacker/css/admin_v3/shared/forms.scss b/app/webpacker/css/admin_v3/shared/forms.scss index 8a37545a29..90ae20cf3f 100644 --- a/app/webpacker/css/admin_v3/shared/forms.scss +++ b/app/webpacker/css/admin_v3/shared/forms.scss @@ -266,11 +266,11 @@ fieldset { } .form-actions { - @include defaultBoxShadow; + box-shadow: $box-shadow; background-color: $fair-pink; border: none; - border-left: 4px solid $red; - border-radius: 4px; + border-left: $border-radius solid $red; + border-radius: $border-radius; margin: 0.5em 0; padding: 0; diff --git a/config/locales/en.yml b/config/locales/en.yml index cedfdfa5c9..9facc4a793 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -851,8 +851,8 @@ en: other: "%{count} products could not be saved. Please review the errors and try again." save: Save changes reset: Discard changes - bulk_update: # TODO: fix these - success: "Products successfully updated" #TODO: add count + bulk_update: + success: Changes saved product_import: title: Product Import file_not_found: File not found or could not be opened @@ -1510,6 +1510,8 @@ en: has_no_payment_methods: "%{enterprise} has no payment methods" has_no_shipping_methods: "%{enterprise} has no shipping methods" has_no_enterprise_fees: "%{enterprise} has no enterprise fees" + flashes: + dismiss: Dismiss side_menu: enterprise: primary_details: "Primary Details" @@ -4341,6 +4343,8 @@ See the %{link} to find out more about %{sitename}'s features and to start using bulk_unit_size: Bulk unit size display_as: display_as: Display As + clone: + success: Product cloned reports: table: select_and_search: "Select filters and click on %{option} to access your data." diff --git a/spec/javascripts/stimulus/flash_controller_test.js b/spec/javascripts/stimulus/flash_controller_test.js new file mode 100644 index 0000000000..665c07aa47 --- /dev/null +++ b/spec/javascripts/stimulus/flash_controller_test.js @@ -0,0 +1,34 @@ +/** + * @jest-environment jsdom + */ + +import { Application } from "stimulus"; +import flash_controller from "../../../app/webpacker/controllers/flash_controller"; + +describe("FlashController", () => { + beforeAll(() => { + const application = Application.start(); + application.register("flash", flash_controller); + }); + + beforeEach(() => { + document.body.innerHTML = ` +
+ `; + + }); + + describe("autoClose", () => { + jest.useFakeTimers(); + + it("is cleared after about 5 seconds", () => { + let element = document.getElementById("element"); + expect(element).not.toBe(null); + + jest.advanceTimersByTime(5500); + + element = document.getElementById("element"); + expect(element).toBe(null); + }); + }); +}); diff --git a/spec/reflexes/products_reflex_spec.rb b/spec/reflexes/products_reflex_spec.rb index debfcd24c8..a79b32b326 100644 --- a/spec/reflexes/products_reflex_spec.rb +++ b/spec/reflexes/products_reflex_spec.rb @@ -7,6 +7,12 @@ describe ProductsReflex, type: :reflex, feature: :admin_style_v3 do let(:context) { { url: admin_products_url, connection: { current_user: } } } + let(:flash) { {} } + + before do + # Mock flash, because stimulus_reflex_testing doesn't support sessions + allow_any_instance_of(described_class).to receive(:flash).and_return(flash) + end describe '#fetch' do subject{ build_reflex(method_name: :fetch, **context) } @@ -54,6 +60,8 @@ describe ProductsReflex, type: :reflex, feature: :admin_style_v3 do product_a.reload }.to change{ product_a.name }.to("Pommes") .and change{ product_a.sku }.to("POM-00") + + expect(flash).to include success: "Changes saved" end it "saves valid changes to products and nested variants" do @@ -89,6 +97,8 @@ describe ProductsReflex, type: :reflex, feature: :admin_style_v3 do .and change{ variant_a1.sku }.to("POM-01") .and change{ variant_a1.price }.to(10.25) .and change{ variant_a1.on_hand }.to(6) + + expect(flash).to include success: "Changes saved" end describe "sorting" do @@ -112,6 +122,7 @@ describe ProductsReflex, type: :reflex, feature: :admin_style_v3 do product_a.id, product_b.id, ] + expect(flash).to include success: "Changes saved" end end @@ -136,6 +147,7 @@ describe ProductsReflex, type: :reflex, feature: :admin_style_v3 do reflex = run_reflex(:bulk_update, params:) expect(reflex.get(:error_counts)).to eq({ saved: 1, invalid: 2 }) + expect(flash).to_not include success: "Changes saved" # # WTF # expect{ reflex(:bulk_update, params:) }.to broadcast( diff --git a/spec/support/request/web_helper.rb b/spec/support/request/web_helper.rb index 10efa75693..4e8aad2be0 100644 --- a/spec/support/request/web_helper.rb +++ b/spec/support/request/web_helper.rb @@ -19,7 +19,7 @@ module WebHelper end def flash_message - find('.flash', visible: false).text(:all).strip + find('.flash .msg', visible: false).text(:all).strip end def handle_js_confirm(accept = true) diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index b4294bdc4d..c34e40b934 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -237,7 +237,6 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 expect(page).to have_css "button[aria-label='On Hand']", text: "6" end - pending expect(page).to have_content "Changes saved" end @@ -259,7 +258,6 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 expect(page).to have_css "button[aria-label='On Hand']", text: "On demand" end - pending expect(page).to have_content "Changes saved" end @@ -298,14 +296,15 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 fill_in "Name", with: "" fill_in "SKU", with: "A" * 256 end + end + + it "shows errors for both product and variant fields" do + # Update variant with invalid data too within row_containing_name("Medium box") do fill_in "Name", with: "L" * 256 fill_in "SKU", with: "1" * 256 fill_in "Price", with: "10.25" end - end - - it "shows errors for both product and variant fields" do # Also update another product with valid data within row_containing_name("Bananas") do fill_in "Name", with: "Bananes" @@ -316,9 +315,7 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 product_a.reload }.to_not change { product_a.name } - # pending("unchanged rows are being saved") # TODO: don't report unchanged rows - # expect(page).to_not have_content("rows were saved correctly") - # Both the product and variant couldn't be saved. + expect(page).to have_content("1 product was saved correctly") expect(page).to have_content("1 product could not be saved") expect(page).to have_content "Please review the errors and try again" @@ -352,7 +349,6 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 }.to change { product_a.name }.to("Pommes") .and change{ product_a.sku }.to("POM-00") - pending expect(page).to have_content "Changes saved" end end @@ -401,6 +397,8 @@ describe 'As an admin, I can see the new product page', feature: :admin_style_v3 click_link('Clone') end + expect(page).to have_content "Product cloned" + within "table.products" do # Gather input values, because page.content doesn't include them. input_content = page.find_all('input[type=text]').map(&:value).join