From a8088ae23186610b587ca4f0f7999774228b291c Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 13 Jun 2023 17:27:48 +0200 Subject: [PATCH 01/49] Create a pseudo element to add a line under the menu item - Increase top menu underline height to 3px - reduce underline width to same width of label --- app/webpacker/css/admin_v3/components/navigation.scss | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/webpacker/css/admin_v3/components/navigation.scss b/app/webpacker/css/admin_v3/components/navigation.scss index ba50629d03..216e376fd1 100644 --- a/app/webpacker/css/admin_v3/components/navigation.scss +++ b/app/webpacker/css/admin_v3/components/navigation.scss @@ -88,7 +88,16 @@ nav.menu { &:hover { color: $red !important; - border-bottom: 2px solid $red; + + &:after { + content: ""; + position: absolute; + bottom: 0; + left: 25px; + right: 25px; + height: 3px; + background: $red; + } } span.text { From 24c045a09cdeddd71767c9d29923bd5ecbb1df4f Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 14 Jun 2023 11:44:10 +0200 Subject: [PATCH 02/49] Reduce padding --- app/webpacker/css/admin_v3/components/navigation.scss | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/webpacker/css/admin_v3/components/navigation.scss b/app/webpacker/css/admin_v3/components/navigation.scss index 216e376fd1..ff1dda3863 100644 --- a/app/webpacker/css/admin_v3/components/navigation.scss +++ b/app/webpacker/css/admin_v3/components/navigation.scss @@ -75,7 +75,7 @@ nav.menu { a { display: block; - padding: 25px 5px; + padding: 16px 5px; color: $dark-grey !important; position: relative; text-align: center; @@ -93,8 +93,8 @@ nav.menu { content: ""; position: absolute; bottom: 0; - left: 25px; - right: 25px; + left: 30px; + right: 30px; height: 3px; background: $red; } From 2892301336fc8254b59994676a9ef14a2fe0e3e7 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Sat, 17 Jun 2023 21:06:47 +0200 Subject: [PATCH 03/49] Factorize menu and submenu into same definition as they are the same, expect the font-size --- .../css/admin_v3/components/navigation.scss | 76 +++++++------------ 1 file changed, 28 insertions(+), 48 deletions(-) diff --git a/app/webpacker/css/admin_v3/components/navigation.scss b/app/webpacker/css/admin_v3/components/navigation.scss index ff1dda3863..7f0badf82f 100644 --- a/app/webpacker/css/admin_v3/components/navigation.scss +++ b/app/webpacker/css/admin_v3/components/navigation.scss @@ -67,49 +67,16 @@ nav.menu { display: flex; } + li a { + font-size: 16px; + } + li { min-width: 90px; flex-grow: 1; padding-left: 2px; padding-right: 2px; - a { - display: block; - padding: 16px 5px; - color: $dark-grey !important; - position: relative; - text-align: center; - font-weight: 600; - font-size: 16px; - - i { - display: inline; - } - - &:hover { - color: $red !important; - - &:after { - content: ""; - position: absolute; - bottom: 0; - left: 30px; - right: 30px; - height: 3px; - background: $red; - } - } - - span.text { - font-weight: 600; - } - } - - a::before { - font-weight: normal; - padding-top: 0; - } - .dropdown { width: 300px; background-color: $teal; @@ -124,31 +91,44 @@ nav.menu { } } } - - &.selected a { - @extend a, :hover; - } } } #sub-menu { padding-bottom: 0; box-shadow: 0px 1px 0px $light-grey; +} +// factoized menu item +#admin-menu, +#sub-menu { li { a { - display: block; - padding: 12px 20px; - color: $dark-grey; + display: inline-block; + padding: 16px 20px; + color: $dark-grey !important; text-align: center; position: relative; font-size: 14px; + font-weight: 600; + + &:hover { + color: $red !important; + + &:after { + content: ""; + position: absolute; + bottom: 0; + left: 20px; + right: 20px; + height: 3px; + background: $red; + } + } } - &.selected a, - a:hover { - color: $red; - border-bottom: 2px solid $red; + &.selected a { + @extend a, :hover; } } } From 1f3d41972fb4a4a7bec701470169be0b62eec84d Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Sat, 17 Jun 2023 21:10:05 +0200 Subject: [PATCH 04/49] Add same padding than #admin-menu,#sub-menu li a in order to align content (header, navigation, content) on both left and right --- .../css/admin_v3/components/navigation.scss | 65 +++++++++++-------- 1 file changed, 37 insertions(+), 28 deletions(-) diff --git a/app/webpacker/css/admin_v3/components/navigation.scss b/app/webpacker/css/admin_v3/components/navigation.scss index 7f0badf82f..95176ba57a 100644 --- a/app/webpacker/css/admin_v3/components/navigation.scss +++ b/app/webpacker/css/admin_v3/components/navigation.scss @@ -64,7 +64,7 @@ nav.menu { box-shadow: 0px 1px 0px rgba(0, 0, 0, 0.05), 0px 2px 2px rgba(0, 0, 0, 0.07); ul { - display: flex; + justify-content: space-between; } li a { @@ -72,11 +72,6 @@ nav.menu { } li { - min-width: 90px; - flex-grow: 1; - padding-left: 2px; - padding-right: 2px; - .dropdown { width: 300px; background-color: $teal; @@ -102,33 +97,42 @@ nav.menu { // factoized menu item #admin-menu, #sub-menu { - li { - a { - display: inline-block; - padding: 16px 20px; - color: $dark-grey !important; - text-align: center; - position: relative; - font-size: 14px; - font-weight: 600; + .container { + padding-left: 10px; + padding-right: 10px; + } - &:hover { - color: $red !important; + ul { + display: flex; - &:after { - content: ""; - position: absolute; - bottom: 0; - left: 20px; - right: 20px; - height: 3px; - background: $red; + li { + a { + display: inline-block; + padding: 16px 20px; + color: $dark-grey !important; + text-align: center; + position: relative; + font-size: 14px; + font-weight: 600; + + &:hover { + color: $red !important; + + &:after { + content: ""; + position: absolute; + bottom: 0; + left: 20px; + right: 20px; + height: 3px; + background: $red; + } } } - } - &.selected a { - @extend a, :hover; + &.selected a { + @extend a, :hover; + } } } } @@ -137,6 +141,11 @@ nav.menu { margin: 0.25em 0; } +#header .container { + padding-left: 30px; + padding-right: 30px; +} + #login-nav { line-height: 1.75em; } From 9762819aa85f749f1216ba873280878f4c8a5330 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 19 Jun 2023 14:46:29 +0200 Subject: [PATCH 05/49] Use the right color --- app/webpacker/css/admin_v3/components/navigation.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/webpacker/css/admin_v3/components/navigation.scss b/app/webpacker/css/admin_v3/components/navigation.scss index 95176ba57a..77e6668558 100644 --- a/app/webpacker/css/admin_v3/components/navigation.scss +++ b/app/webpacker/css/admin_v3/components/navigation.scss @@ -109,7 +109,7 @@ nav.menu { a { display: inline-block; padding: 16px 20px; - color: $dark-grey !important; + color: $color-9 !important; text-align: center; position: relative; font-size: 14px; From 9b92c25879ad86c27d02c7a4fa7426f26e24c3b6 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 20 Jun 2023 11:00:18 +0200 Subject: [PATCH 06/49] Add a section for specific rules for both admin-menu and sub-menu + change height for border bottom on selected/hovered menu --- .../css/admin_v3/components/navigation.scss | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/app/webpacker/css/admin_v3/components/navigation.scss b/app/webpacker/css/admin_v3/components/navigation.scss index 77e6668558..b7cf37e9ca 100644 --- a/app/webpacker/css/admin_v3/components/navigation.scss +++ b/app/webpacker/css/admin_v3/components/navigation.scss @@ -63,14 +63,6 @@ nav.menu { #admin-menu { box-shadow: 0px 1px 0px rgba(0, 0, 0, 0.05), 0px 2px 2px rgba(0, 0, 0, 0.07); - ul { - justify-content: space-between; - } - - li a { - font-size: 16px; - } - li { .dropdown { width: 300px; @@ -94,7 +86,7 @@ nav.menu { box-shadow: 0px 1px 0px $light-grey; } -// factoized menu item +// Factorized rules on menu item for admin menu and sub menu #admin-menu, #sub-menu { .container { @@ -137,6 +129,24 @@ nav.menu { } } +// Specific rules on menu item for admin menu and sub menu +#admin-menu { + ul { + justify-content: space-between; + li a { + font-size: 16px; + } + } +} + +#sub-menu { + ul li a:hover { + &:after { + height: 2px; + } + } +} + #header figure { margin: 0.25em 0; } From 69cff855770547728ec44d1e7e8e1d05bdd84141 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 20 Jun 2023 11:10:19 +0200 Subject: [PATCH 07/49] Change color to EFF1F2 --- app/webpacker/css/admin_v3/components/navigation.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/webpacker/css/admin_v3/components/navigation.scss b/app/webpacker/css/admin_v3/components/navigation.scss index b7cf37e9ca..01054103af 100644 --- a/app/webpacker/css/admin_v3/components/navigation.scss +++ b/app/webpacker/css/admin_v3/components/navigation.scss @@ -83,7 +83,7 @@ nav.menu { #sub-menu { padding-bottom: 0; - box-shadow: 0px 1px 0px $light-grey; + box-shadow: 0px 1px 0px $color-7; } // Factorized rules on menu item for admin menu and sub menu From 47f21cb59ebdee58f06389e49bc047146f71ea0e Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 21 Jun 2023 11:00:33 +0200 Subject: [PATCH 08/49] Add pagination with pagy For `admin_style_v3` use `<` and `>` for next and previous link instead of `next` and `previous` string Extract a mixin for a default box-shadow Maybe this needs to be redefined. Let's see how next things goes. When a table is followed by a pagination, remove its margin-bottom + border Finally, design the pagination component Add sorting/pagination module, on top of table We use `cablea_ready.replace`, so need to add `#products-content` id Use a `pagy` partial with reflex action, instead of the legacy one - revert the legacy one to its previous state - in reflex, fetch product with page attribute, 1 by default Move `pagy` into `admin/shared/v3/` to be reusable + use fontawesome icons for next and previous page Remove useless line --- app/reflexes/admin/products_v3_reflex.rb | 11 +++++--- .../admin/products_v3/_content.html.haml | 25 +++++++++++------- app/views/admin/products_v3/_sort.html.haml | 2 ++ app/views/admin/shared/v3/_pagy.html.haml | 21 +++++++++++++++ app/webpacker/css/admin/products_v3.scss | 4 +++ app/webpacker/css/admin_v3/all.scss | 1 + .../css/admin_v3/components/navigation.scss | 2 +- .../css/admin_v3/components/pagination.scss | 26 +++++++++++++++---- app/webpacker/css/admin_v3/mixins.scss | 3 +++ app/webpacker/css/admin_v3/shared/tables.scss | 4 +++ config/locales/en.yml | 3 +++ 11 files changed, 82 insertions(+), 20 deletions(-) create mode 100644 app/views/admin/products_v3/_sort.html.haml create mode 100644 app/views/admin/shared/v3/_pagy.html.haml create mode 100644 app/webpacker/css/admin_v3/mixins.scss diff --git a/app/reflexes/admin/products_v3_reflex.rb b/app/reflexes/admin/products_v3_reflex.rb index 0e36ae2762..0bfc63431a 100644 --- a/app/reflexes/admin/products_v3_reflex.rb +++ b/app/reflexes/admin/products_v3_reflex.rb @@ -2,12 +2,15 @@ module Admin class ProductsV3Reflex < ApplicationReflex - before_reflex :fetch_products, only: [:fetch] + include Pagy::Backend def fetch + fetch_products(element.dataset.page || 1) + cable_ready.replace( selector: "#products-content", - html: render(partial: "admin/products_v3/content", locals: { products: @products }) + html: render(partial: "admin/products_v3/content", + locals: { products: @products, pagy: @pagy }) ).broadcast morph :nothing @@ -16,10 +19,10 @@ module Admin private # copied from ProductsTableComponent - def fetch_products + def fetch_products(page) product_query = OpenFoodNetwork::Permissions.new(current_user) .editable_products.merge(product_scope) - @products = product_query.order(:name).limit(50) + @pagy, @products = pagy(product_query.order(:name), items: 15, page:) end def product_scope diff --git a/app/views/admin/products_v3/_content.html.haml b/app/views/admin/products_v3/_content.html.haml index 2c076973c6..8ac13d94eb 100644 --- a/app/views/admin/products_v3/_content.html.haml +++ b/app/views/admin/products_v3/_content.html.haml @@ -1,10 +1,15 @@ -- if products.any? - = render partial: 'table', locals: { products: products } -- else - #no-products - = t('.no_products_found') - #no-products-actions - %a{ href: "/admin/products/new", class: "button icon-plus", icon: "icon-plus" } - = t(:new_product) - %a{ href: "/admin/products/import", class: "button icon-upload secondary", icon: "icon-upload" } - = t(".import_products") +#products-content + - if products.any? + .container + .sixteen.columns + = render partial: 'sort', locals: { pagy: pagy } + = render partial: 'table', locals: { products: products } + = render partial: 'admin/shared/v3/pagy', locals: { pagy: pagy, reflex: "click->Admin::ProductsV3#fetch" } + - else + #no-products + = t('.no_products_found') + #no-products-actions + %a{ href: "/admin/products/new", class: "button icon-plus", icon: "icon-plus" } + = t(:new_product) + %a{ href: "/admin/products/import", class: "button icon-upload secondary", icon: "icon-upload" } + = t(".import_products") diff --git a/app/views/admin/products_v3/_sort.html.haml b/app/views/admin/products_v3/_sort.html.haml new file mode 100644 index 0000000000..607d99596b --- /dev/null +++ b/app/views/admin/products_v3/_sort.html.haml @@ -0,0 +1,2 @@ +#sort + = t(".pagination.total_html", total: pagy.count, from: pagy.from, to: pagy.to) diff --git a/app/views/admin/shared/v3/_pagy.html.haml b/app/views/admin/shared/v3/_pagy.html.haml new file mode 100644 index 0000000000..016b1c2f2f --- /dev/null +++ b/app/views/admin/shared/v3/_pagy.html.haml @@ -0,0 +1,21 @@ +%nav.pagy_nav.pagination{"aria-label" => "pager", :role => "navigation"} + - if pagy.prev + %a.page.previous{ href: "#", "data-reflex": reflex, "data-page": pagy.prev || 1, "aria-label": "previous"} + %i.icon-chevron-left + - else + %span.page.prev.disabled + %i.icon-chevron-left + - pagy.series.each do |item| # series example: [1, :gap, 7, 8, "9", 10, 11, :gap, 36] + - if item.is_a?(Integer) # page link + %a.page{ href: "#", "data-reflex": reflex, "data-page": item, "aria-label": "page #{item}"} + = item + - elsif item.is_a?(String) # current page + %span.page.current= item + - elsif item == :gap # page gap + %span.page.gap … + - if pagy.next + %a.page.next{ href: "#", "data-reflex": reflex, "data-page": pagy.next || pagy.last, "aria-label": "next"} + %i.icon-chevron-right + - else + %span.page.next.disabled + %i.icon-chevron-right diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss index 7050cd9983..2b19cfd506 100644 --- a/app/webpacker/css/admin/products_v3.scss +++ b/app/webpacker/css/admin/products_v3.scss @@ -78,4 +78,8 @@ gap: 20px; } } + + #sort { + margin-bottom: 1em; + } } diff --git a/app/webpacker/css/admin_v3/all.scss b/app/webpacker/css/admin_v3/all.scss index 2a56c8a6cc..0655e5773b 100644 --- a/app/webpacker/css/admin_v3/all.scss +++ b/app/webpacker/css/admin_v3/all.scss @@ -22,6 +22,7 @@ @import "globals/variables"; // admin_v3 @import "../admin/variables"; @import "../admin/globals/mixins"; +@import "mixins"; // admin_v3 @import "../admin/plugins/font-awesome"; diff --git a/app/webpacker/css/admin_v3/components/navigation.scss b/app/webpacker/css/admin_v3/components/navigation.scss index 01054103af..6b5a2518ae 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 { - box-shadow: 0px 1px 0px rgba(0, 0, 0, 0.05), 0px 2px 2px rgba(0, 0, 0, 0.07); + @include defaultBoxShadow; li { .dropdown { diff --git a/app/webpacker/css/admin_v3/components/pagination.scss b/app/webpacker/css/admin_v3/components/pagination.scss index dc269c4227..53167e9996 100644 --- a/app/webpacker/css/admin_v3/components/pagination.scss +++ b/app/webpacker/css/admin_v3/components/pagination.scss @@ -1,21 +1,37 @@ .pagination { text-align: center; - margin: 2em 0 1em; + margin: 0 0 1em; padding: 10px 0; - background-color: $light-grey; + background-color: $color-7; .page { - padding: 5px 8px; + width: 40px; + line-height: 40px; text-align: center; display: inline-block; text-align: center; + background-color: $color-1; + @include defaultBoxShadow; + border-radius: 4px; + color: $color-9; &.current { - background-color: $green; - border-radius: 3px; + background-color: $color-5; color: $white; } + + &.prev { + margin-right: 20px; + } + + &.next { + margin-left: 20px; + } + + &.disabled { + cursor: default; + } } button { diff --git a/app/webpacker/css/admin_v3/mixins.scss b/app/webpacker/css/admin_v3/mixins.scss new file mode 100644 index 0000000000..76f01458be --- /dev/null +++ b/app/webpacker/css/admin_v3/mixins.scss @@ -0,0 +1,3 @@ +@mixin defaultBoxShadow { + box-shadow: 0px 1px 0px rgba(0, 0, 0, 0.05), 0px 2px 2px rgba(0, 0, 0, 0.07); +} diff --git a/app/webpacker/css/admin_v3/shared/tables.scss b/app/webpacker/css/admin_v3/shared/tables.scss index 4b493c8acb..0611bd80da 100644 --- a/app/webpacker/css/admin_v3/shared/tables.scss +++ b/app/webpacker/css/admin_v3/shared/tables.scss @@ -205,3 +205,7 @@ table { } } } + +table + .pagination { + margin-top: -18px; +} diff --git a/config/locales/en.yml b/config/locales/en.yml index 5eda42785d..30a073dcdf 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -767,6 +767,9 @@ en: header: title: Bulk Edit Products loading: Loading your products + sort: + pagination: + total_html: "%{total} products in your catalogue. Showing %{from} to %{to}." content: no_products_found: No products found import_products: Import multiple products From 3b615086526b8c1b5c434ec9627988ca2637271a Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 26 Jun 2023 16:54:06 +0200 Subject: [PATCH 09/49] Be more specific to only apply to the first container, ie. the root one + Override default position (relative) in order to have loading above --- app/webpacker/css/admin/products_v3.scss | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss index 2b19cfd506..98a5463499 100644 --- a/app/webpacker/css/admin/products_v3.scss +++ b/app/webpacker/css/admin/products_v3.scss @@ -1,12 +1,16 @@ // Customisations for the new Bulk Edit Products page only .products_v3_page { - #content .container { + #content > .row:first-child > .container:first-child { // Allow table to extend to full width of available screen space // TODO: move this to a generic rule, eg body.full-width{}. Then it can be included on any page. // or even better, create a switch that allows you to yield the page content without the surrounding content class. then you still have control to add the .content div where needed. max-width: none; } + #products-content > .container:first-child { + position: static; + } + // Hopefully these rules will be moved to component(s). table.products { table-layout: fixed; // Column widths are based solely on col definitions (not content). This allows more efficient rendering. From 5aebbe410285475f271c8b01fa67a52604760255 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 27 Jun 2023 11:44:36 +0200 Subject: [PATCH 10/49] use `beforeFetch` and `afterFetch` lifecycle methods + Move outside `Admin` module the reflex Therefore, this reflex should be _equivalent_ to its javascript controller: `ProductsV3` (relation is made through names) Remove unwanted line Actually call StimulusJS controller instead of calling the reflex itself In order to have this "showLoading", "hideLoading" behavior. It seems to be possible to directly use the Reflex itself (use `data-reflex` instead of `data-action`) but I can't make it work: the `stimulus-controller:after` event is never broadcasted/catched (but `stimulus-controller:before` yes...) Documentation: https://docs.stimulusreflex.com/guide/reflexes.html#understanding-stimulusreflex-controllers https://docs.stimulusreflex.com/guide/lifecycle.html#generic-life-cycle-methods Maybe @dacook if you want to have a look... --- app/reflexes/admin/products_v3_reflex.rb | 51 ------------------- app/reflexes/products_v3_reflex.rb | 49 ++++++++++++++++++ .../admin/products_v3/_content.html.haml | 2 +- app/views/admin/shared/v3/_pagy.html.haml | 6 +-- .../controllers/productsV3_controller.js | 19 +++++-- 5 files changed, 68 insertions(+), 59 deletions(-) delete mode 100644 app/reflexes/admin/products_v3_reflex.rb create mode 100644 app/reflexes/products_v3_reflex.rb diff --git a/app/reflexes/admin/products_v3_reflex.rb b/app/reflexes/admin/products_v3_reflex.rb deleted file mode 100644 index 0bfc63431a..0000000000 --- a/app/reflexes/admin/products_v3_reflex.rb +++ /dev/null @@ -1,51 +0,0 @@ -# frozen_string_literal: true - -module Admin - class ProductsV3Reflex < ApplicationReflex - include Pagy::Backend - - def fetch - fetch_products(element.dataset.page || 1) - - cable_ready.replace( - selector: "#products-content", - html: render(partial: "admin/products_v3/content", - locals: { products: @products, pagy: @pagy }) - ).broadcast - - morph :nothing - end - - private - - # copied from ProductsTableComponent - def fetch_products(page) - product_query = OpenFoodNetwork::Permissions.new(current_user) - .editable_products.merge(product_scope) - @pagy, @products = pagy(product_query.order(:name), items: 15, page:) - end - - def product_scope - scope = if current_user.has_spree_role?("admin") || current_user.enterprises.present? - Spree::Product - else - Spree::Product.active - end - - scope.includes(product_query_includes) - end - - # Optimise by pre-loading required columns - def product_query_includes - # TODO: add other fields used in columns? (eg supplier: [:name]) - [ - # variants: [ - # :default_price, - # :stock_locations, - # :stock_items, - # :variant_overrides - # ] - ] - end - end -end diff --git a/app/reflexes/products_v3_reflex.rb b/app/reflexes/products_v3_reflex.rb new file mode 100644 index 0000000000..79e7a6055e --- /dev/null +++ b/app/reflexes/products_v3_reflex.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +class ProductsV3Reflex < ApplicationReflex + include Pagy::Backend + + def fetch + fetch_products(element.dataset.page || 1, element.dataset.per_page || 15) + + cable_ready.replace( + selector: "#products-content", + html: render(partial: "admin/products_v3/content", + locals: { products: @products, pagy: @pagy }) + ).broadcast + + morph :nothing + end + + private + + # copied from ProductsTableComponent + def fetch_products(page, per_page) + product_query = OpenFoodNetwork::Permissions.new(current_user) + .editable_products.merge(product_scope) + @pagy, @products = pagy(product_query.order(:name), items: per_page, page:) + end + + def product_scope + scope = if current_user.has_spree_role?("admin") || current_user.enterprises.present? + Spree::Product + else + Spree::Product.active + end + + scope.includes(product_query_includes) + end + + # Optimise by pre-loading required columns + def product_query_includes + # TODO: add other fields used in columns? (eg supplier: [:name]) + [ + # variants: [ + # :default_price, + # :stock_locations, + # :stock_items, + # :variant_overrides + # ] + ] + end +end diff --git a/app/views/admin/products_v3/_content.html.haml b/app/views/admin/products_v3/_content.html.haml index 8ac13d94eb..fd94e0fd91 100644 --- a/app/views/admin/products_v3/_content.html.haml +++ b/app/views/admin/products_v3/_content.html.haml @@ -4,7 +4,7 @@ .sixteen.columns = render partial: 'sort', locals: { pagy: pagy } = render partial: 'table', locals: { products: products } - = render partial: 'admin/shared/v3/pagy', locals: { pagy: pagy, reflex: "click->Admin::ProductsV3#fetch" } + = render partial: 'admin/shared/v3/pagy', locals: { pagy: pagy, action: "click->productsV3#fetch" } - else #no-products = t('.no_products_found') diff --git a/app/views/admin/shared/v3/_pagy.html.haml b/app/views/admin/shared/v3/_pagy.html.haml index 016b1c2f2f..03a1c65016 100644 --- a/app/views/admin/shared/v3/_pagy.html.haml +++ b/app/views/admin/shared/v3/_pagy.html.haml @@ -1,20 +1,20 @@ %nav.pagy_nav.pagination{"aria-label" => "pager", :role => "navigation"} - if pagy.prev - %a.page.previous{ href: "#", "data-reflex": reflex, "data-page": pagy.prev || 1, "aria-label": "previous"} + %a.page.prev{ href: "#", "data-action": action, "data-page": pagy.prev || 1, "aria-label": "previous"} %i.icon-chevron-left - else %span.page.prev.disabled %i.icon-chevron-left - pagy.series.each do |item| # series example: [1, :gap, 7, 8, "9", 10, 11, :gap, 36] - if item.is_a?(Integer) # page link - %a.page{ href: "#", "data-reflex": reflex, "data-page": item, "aria-label": "page #{item}"} + %a.page{ href: "#", "data-action": action, "data-page": item, "aria-label": "page #{item}"} = item - elsif item.is_a?(String) # current page %span.page.current= item - elsif item == :gap # page gap %span.page.gap … - if pagy.next - %a.page.next{ href: "#", "data-reflex": reflex, "data-page": pagy.next || pagy.last, "aria-label": "next"} + %a.page.next{ href: "#", "data-action": action, "data-page": pagy.next || pagy.last, "aria-label": "next"} %i.icon-chevron-right - else %span.page.next.disabled diff --git a/app/webpacker/controllers/productsV3_controller.js b/app/webpacker/controllers/productsV3_controller.js index 9a034b69e0..4e783f26c3 100644 --- a/app/webpacker/controllers/productsV3_controller.js +++ b/app/webpacker/controllers/productsV3_controller.js @@ -6,12 +6,15 @@ export default class extends ApplicationController { connect() { super.connect(); // Fetch the products on page load - this.load(); + this.fetch(); } - load = () => { - this.showLoading(); - this.stimulate("Admin::ProductsV3#fetch").then(() => this.hideLoading()); + fetch = (event = {}) => { + if (event && event.target) { + this.stimulate("ProductsV3#fetch", event.target); + return; + } + this.stimulate("ProductsV3#fetch"); }; hideLoading = () => { @@ -21,4 +24,12 @@ export default class extends ApplicationController { showLoading = () => { this.loadingTarget.classList.remove("hidden"); }; + + beforeFetch(element, reflex, noop, reflexId) { + this.showLoading(); + } + + afterFetch(element, reflex, noop, reflexId) { + this.hideLoading(); + } } From 0b83dc088df709a0d3feea7d9b361dbb634042ba Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 27 Jun 2023 14:58:27 +0200 Subject: [PATCH 11/49] Separation of concerns: use a loading controller Two different events can be used: `show-loading` and `hide-loading`. I'm not 100% sure this is the right way to go, but let's try! --- app/views/admin/products_v3/index.html.haml | 2 +- .../controllers/loading_controller.js | 22 +++++++++++++++++++ .../controllers/productsV3_controller.js | 16 ++++---------- 3 files changed, 27 insertions(+), 13 deletions(-) create mode 100644 app/webpacker/controllers/loading_controller.js diff --git a/app/views/admin/products_v3/index.html.haml b/app/views/admin/products_v3/index.html.haml index 2cb442b141..4c92723599 100644 --- a/app/views/admin/products_v3/index.html.haml +++ b/app/views/admin/products_v3/index.html.haml @@ -11,7 +11,7 @@ = render partial: 'spree/admin/shared/product_sub_menu' #products_v3_page{"data-controller": "productsV3"} - #loading-spinner.spinner-container{"data-productsV3-target": "loading"} + #loading-spinner.spinner-container{ "data-controller": "loading" } .spinner = t('.loading') #products-content diff --git a/app/webpacker/controllers/loading_controller.js b/app/webpacker/controllers/loading_controller.js new file mode 100644 index 0000000000..903cc9898d --- /dev/null +++ b/app/webpacker/controllers/loading_controller.js @@ -0,0 +1,22 @@ +import ApplicationController from "./application_controller"; + +export default class extends ApplicationController { + connect() { + super.connect(); + document.addEventListener("show-loading", this.showLoading); + document.addEventListener("hide-loading", this.hideLoading); + } + + disconnect() { + document.removeEventListener("show-loading", this.showLoading); + document.removeEventListener("hide-loading", this.hideLoading); + } + + hideLoading = () => { + this.element.classList.add("hidden"); + }; + + showLoading = () => { + this.element.classList.remove("hidden"); + }; +} diff --git a/app/webpacker/controllers/productsV3_controller.js b/app/webpacker/controllers/productsV3_controller.js index 4e783f26c3..62b653c4db 100644 --- a/app/webpacker/controllers/productsV3_controller.js +++ b/app/webpacker/controllers/productsV3_controller.js @@ -1,8 +1,6 @@ import ApplicationController from "./application_controller"; export default class extends ApplicationController { - static targets = ["loading"]; - connect() { super.connect(); // Fetch the products on page load @@ -17,19 +15,13 @@ export default class extends ApplicationController { this.stimulate("ProductsV3#fetch"); }; - hideLoading = () => { - this.loadingTarget.classList.add("hidden"); - }; - - showLoading = () => { - this.loadingTarget.classList.remove("hidden"); - }; - beforeFetch(element, reflex, noop, reflexId) { - this.showLoading(); + const event = new CustomEvent("show-loading"); + document.dispatchEvent(event); } afterFetch(element, reflex, noop, reflexId) { - this.hideLoading(); + const event = new CustomEvent("hide-loading"); + document.dispatchEvent(event); } } From 574adb88d2f031a86c2b9bd92ab1a62d41af7a76 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 27 Jun 2023 16:12:44 +0200 Subject: [PATCH 12/49] Add a per page component This is still a WIP. --- app/reflexes/products_v3_reflex.rb | 26 ++++++++++++---- app/views/admin/products_v3/_sort.html.haml | 6 +++- .../controllers/productsV3_controller.js | 30 +++++++++++++++---- app/webpacker/css/admin/products_v3.scss | 10 +++++++ config/locales/en.yml | 3 ++ 5 files changed, 64 insertions(+), 11 deletions(-) diff --git a/app/reflexes/products_v3_reflex.rb b/app/reflexes/products_v3_reflex.rb index 79e7a6055e..c81ef81eba 100644 --- a/app/reflexes/products_v3_reflex.rb +++ b/app/reflexes/products_v3_reflex.rb @@ -4,8 +4,26 @@ class ProductsV3Reflex < ApplicationReflex include Pagy::Backend def fetch - fetch_products(element.dataset.page || 1, element.dataset.per_page || 15) + @page = element.dataset.page || 1 + @per_page ||= 15 + fetch_products + + render_products + end + + def change_per_page + @per_page = element.value.to_i + @page = 1 + + fetch_products + + render_products + end + + private + + def render_products cable_ready.replace( selector: "#products-content", html: render(partial: "admin/products_v3/content", @@ -15,13 +33,11 @@ class ProductsV3Reflex < ApplicationReflex morph :nothing end - private - # copied from ProductsTableComponent - def fetch_products(page, per_page) + def fetch_products product_query = OpenFoodNetwork::Permissions.new(current_user) .editable_products.merge(product_scope) - @pagy, @products = pagy(product_query.order(:name), items: per_page, page:) + @pagy, @products = pagy(product_query.order(:name), items: @per_page, page: @page) end def product_scope diff --git a/app/views/admin/products_v3/_sort.html.haml b/app/views/admin/products_v3/_sort.html.haml index 607d99596b..393c426cb1 100644 --- a/app/views/admin/products_v3/_sort.html.haml +++ b/app/views/admin/products_v3/_sort.html.haml @@ -1,2 +1,6 @@ #sort - = t(".pagination.total_html", total: pagy.count, from: pagy.from, to: pagy.to) + %div + = t(".pagination.total_html", total: pagy.count, from: pagy.from, to: pagy.to) + %div.with-dropdown + = t(".pagination.per_page.show") + = select_tag :per_page, options_for_select([15, 25, 50, 100].collect{|i| [t('.pagination.per_page.per_page', num: i), i]}, pagy.items), data: { action: "change->productsV3#changePerPage" } diff --git a/app/webpacker/controllers/productsV3_controller.js b/app/webpacker/controllers/productsV3_controller.js index 62b653c4db..da2dff341f 100644 --- a/app/webpacker/controllers/productsV3_controller.js +++ b/app/webpacker/controllers/productsV3_controller.js @@ -15,13 +15,33 @@ export default class extends ApplicationController { this.stimulate("ProductsV3#fetch"); }; - beforeFetch(element, reflex, noop, reflexId) { - const event = new CustomEvent("show-loading"); - document.dispatchEvent(event); + changePerPage = (event) => { + this.stimulate("ProductsV3#change_per_page", event.target); + }; + + beforeChangePerPage() { + this.showLoading(); } - afterFetch(element, reflex, noop, reflexId) { + afterChangePerPage() { + this.hideLoading(); + } + + beforeFetch() { + this.showLoading(); + } + + afterFetch() { + this.hideLoading(); + } + + showLoading = () => { + const event = new CustomEvent("show-loading"); + document.dispatchEvent(event); + }; + + hideLoading = () => { const event = new CustomEvent("hide-loading"); document.dispatchEvent(event); - } + }; } diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss index 98a5463499..7eb5a11411 100644 --- a/app/webpacker/css/admin/products_v3.scss +++ b/app/webpacker/css/admin/products_v3.scss @@ -85,5 +85,15 @@ #sort { margin-bottom: 1em; + display: flex; + justify-content: space-between; + align-items: center; + + .with-dropdown { + display: flex; + justify-content: space-between; + align-items: center; + gap: 10px; + } } } diff --git a/config/locales/en.yml b/config/locales/en.yml index 30a073dcdf..acc3392d23 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -770,6 +770,9 @@ en: sort: pagination: total_html: "%{total} products in your catalogue. Showing %{from} to %{to}." + per_page: + show: Show + per_page: "%{num} per page" content: no_products_found: No products found import_products: Import multiple products From 61d1f30e0424e8388d3ded513babec90354f9696 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Thu, 29 Jun 2023 16:13:56 +0200 Subject: [PATCH 13/49] Rename `productsV3` to `products`: avoid upper|lower case issues This is causing some issues, misunderstandings around case. Therefore, directly use the Reflex, and lifecycle methods `beforeFetch` and `afterFetch` are called, even if we use the Reflex (and not the js controller methods). This is pretty handy. + adds some id to pagination element (https://docs.stimulusreflex.com/guide/reflexes.html#declaring-a-reflex-in-html-with-data-attributes) Global documentation: https://docs.stimulusreflex.com/guide/reflexes.html --- app/reflexes/{products_v3_reflex.rb => products_reflex.rb} | 2 +- app/views/admin/products_v3/_content.html.haml | 2 +- app/views/admin/products_v3/_sort.html.haml | 2 +- app/views/admin/products_v3/index.html.haml | 2 +- app/views/admin/shared/v3/_pagy.html.haml | 6 +++--- .../{productsV3_controller.js => products_controller.js} | 6 +++--- 6 files changed, 10 insertions(+), 10 deletions(-) rename app/reflexes/{products_v3_reflex.rb => products_reflex.rb} (96%) rename app/webpacker/controllers/{productsV3_controller.js => products_controller.js} (82%) diff --git a/app/reflexes/products_v3_reflex.rb b/app/reflexes/products_reflex.rb similarity index 96% rename from app/reflexes/products_v3_reflex.rb rename to app/reflexes/products_reflex.rb index c81ef81eba..23934f70f5 100644 --- a/app/reflexes/products_v3_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -class ProductsV3Reflex < ApplicationReflex +class ProductsReflex < ApplicationReflex include Pagy::Backend def fetch diff --git a/app/views/admin/products_v3/_content.html.haml b/app/views/admin/products_v3/_content.html.haml index fd94e0fd91..247e51b9a6 100644 --- a/app/views/admin/products_v3/_content.html.haml +++ b/app/views/admin/products_v3/_content.html.haml @@ -4,7 +4,7 @@ .sixteen.columns = render partial: 'sort', locals: { pagy: pagy } = render partial: 'table', locals: { products: products } - = render partial: 'admin/shared/v3/pagy', locals: { pagy: pagy, action: "click->productsV3#fetch" } + = render partial: 'admin/shared/v3/pagy', locals: { pagy: pagy, reflex: "click->Products#fetch" } - else #no-products = t('.no_products_found') diff --git a/app/views/admin/products_v3/_sort.html.haml b/app/views/admin/products_v3/_sort.html.haml index 393c426cb1..c20f696851 100644 --- a/app/views/admin/products_v3/_sort.html.haml +++ b/app/views/admin/products_v3/_sort.html.haml @@ -3,4 +3,4 @@ = t(".pagination.total_html", total: pagy.count, from: pagy.from, to: pagy.to) %div.with-dropdown = t(".pagination.per_page.show") - = select_tag :per_page, options_for_select([15, 25, 50, 100].collect{|i| [t('.pagination.per_page.per_page', num: i), i]}, pagy.items), data: { action: "change->productsV3#changePerPage" } + = select_tag :per_page, options_for_select([15, 25, 50, 100].collect{|i| [t('.pagination.per_page.per_page', num: i), i]}, pagy.items), data: { action: "change->products#changePerPage" } diff --git a/app/views/admin/products_v3/index.html.haml b/app/views/admin/products_v3/index.html.haml index 4c92723599..54dd7f363a 100644 --- a/app/views/admin/products_v3/index.html.haml +++ b/app/views/admin/products_v3/index.html.haml @@ -10,7 +10,7 @@ = render partial: 'spree/admin/shared/product_sub_menu' -#products_v3_page{"data-controller": "productsV3"} +#products_v3_page{"data-controller": "products"} #loading-spinner.spinner-container{ "data-controller": "loading" } .spinner = t('.loading') diff --git a/app/views/admin/shared/v3/_pagy.html.haml b/app/views/admin/shared/v3/_pagy.html.haml index 03a1c65016..68ae3aad56 100644 --- a/app/views/admin/shared/v3/_pagy.html.haml +++ b/app/views/admin/shared/v3/_pagy.html.haml @@ -1,20 +1,20 @@ %nav.pagy_nav.pagination{"aria-label" => "pager", :role => "navigation"} - if pagy.prev - %a.page.prev{ href: "#", "data-action": action, "data-page": pagy.prev || 1, "aria-label": "previous"} + %a.page.prev{ href: "#", id: "pagy-prev", "data-reflex": reflex, "data-page": pagy.prev || 1, "aria-label": "previous"} %i.icon-chevron-left - else %span.page.prev.disabled %i.icon-chevron-left - pagy.series.each do |item| # series example: [1, :gap, 7, 8, "9", 10, 11, :gap, 36] - if item.is_a?(Integer) # page link - %a.page{ href: "#", "data-action": action, "data-page": item, "aria-label": "page #{item}"} + %a.page{ href: "#", id:"pagy-#{item}", "data-reflex": reflex, "data-page": item, "aria-label": "page #{item}"} = item - elsif item.is_a?(String) # current page %span.page.current= item - elsif item == :gap # page gap %span.page.gap … - if pagy.next - %a.page.next{ href: "#", "data-action": action, "data-page": pagy.next || pagy.last, "aria-label": "next"} + %a.page.next{ href: "#", id:"pagy-next", "data-reflex": reflex, "data-page": pagy.next || pagy.last, "aria-label": "next"} %i.icon-chevron-right - else %span.page.next.disabled diff --git a/app/webpacker/controllers/productsV3_controller.js b/app/webpacker/controllers/products_controller.js similarity index 82% rename from app/webpacker/controllers/productsV3_controller.js rename to app/webpacker/controllers/products_controller.js index da2dff341f..681b92b42c 100644 --- a/app/webpacker/controllers/productsV3_controller.js +++ b/app/webpacker/controllers/products_controller.js @@ -9,14 +9,14 @@ export default class extends ApplicationController { fetch = (event = {}) => { if (event && event.target) { - this.stimulate("ProductsV3#fetch", event.target); + this.stimulate("Products#fetch", event.target); return; } - this.stimulate("ProductsV3#fetch"); + this.stimulate("Products#fetch"); }; changePerPage = (event) => { - this.stimulate("ProductsV3#change_per_page", event.target); + this.stimulate("Products#change_per_page", event.target); }; beforeChangePerPage() { From 3da6e0219269587325c66299341cc15e257dc60c Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Thu, 29 Jun 2023 16:27:23 +0200 Subject: [PATCH 14/49] Call pagination via `perPage` param: no state to manage, use param --- app/reflexes/products_reflex.rb | 8 +++----- app/views/admin/shared/v3/_pagy.html.haml | 6 +++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 23934f70f5..c3e7a54cb2 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -4,8 +4,8 @@ class ProductsReflex < ApplicationReflex include Pagy::Backend def fetch - @page = element.dataset.page || 1 - @per_page ||= 15 + @page ||= element.dataset.page || 1 + @per_page ||= element.dataset.perpage || 15 fetch_products @@ -16,9 +16,7 @@ class ProductsReflex < ApplicationReflex @per_page = element.value.to_i @page = 1 - fetch_products - - render_products + fetch end private diff --git a/app/views/admin/shared/v3/_pagy.html.haml b/app/views/admin/shared/v3/_pagy.html.haml index 68ae3aad56..3ba80ea464 100644 --- a/app/views/admin/shared/v3/_pagy.html.haml +++ b/app/views/admin/shared/v3/_pagy.html.haml @@ -1,20 +1,20 @@ %nav.pagy_nav.pagination{"aria-label" => "pager", :role => "navigation"} - if pagy.prev - %a.page.prev{ href: "#", id: "pagy-prev", "data-reflex": reflex, "data-page": pagy.prev || 1, "aria-label": "previous"} + %a.page.prev{ href: "#", id: "pagy-prev", "data-reflex": reflex, "data-perPage": pagy.items, "data-page": pagy.prev || 1, "aria-label": "previous"} %i.icon-chevron-left - else %span.page.prev.disabled %i.icon-chevron-left - pagy.series.each do |item| # series example: [1, :gap, 7, 8, "9", 10, 11, :gap, 36] - if item.is_a?(Integer) # page link - %a.page{ href: "#", id:"pagy-#{item}", "data-reflex": reflex, "data-page": item, "aria-label": "page #{item}"} + %a.page{ href: "#", id:"pagy-#{item}", "data-reflex": reflex, "data-perPage": pagy.items, "data-page": item, "aria-label": "page #{item}"} = item - elsif item.is_a?(String) # current page %span.page.current= item - elsif item == :gap # page gap %span.page.gap … - if pagy.next - %a.page.next{ href: "#", id:"pagy-next", "data-reflex": reflex, "data-page": pagy.next || pagy.last, "aria-label": "next"} + %a.page.next{ href: "#", id:"pagy-next", "data-reflex": reflex, "data-perPage": pagy.items, "data-page": pagy.next || pagy.last, "aria-label": "next"} %i.icon-chevron-right - else %span.page.next.disabled From 0fd4d892d9c37877b0e6ca2eca5ee21e7f572aaa Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Thu, 29 Jun 2023 17:29:11 +0200 Subject: [PATCH 15/49] This is now unused: through the js controller only at startup --- app/webpacker/controllers/products_controller.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/webpacker/controllers/products_controller.js b/app/webpacker/controllers/products_controller.js index 681b92b42c..ab65d2a1bf 100644 --- a/app/webpacker/controllers/products_controller.js +++ b/app/webpacker/controllers/products_controller.js @@ -7,11 +7,7 @@ export default class extends ApplicationController { this.fetch(); } - fetch = (event = {}) => { - if (event && event.target) { - this.stimulate("Products#fetch", event.target); - return; - } + fetch = () => { this.stimulate("Products#fetch"); }; From c786f300cae2bf13c8e2516b6cfefe735f720c28 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Thu, 29 Jun 2023 17:34:43 +0200 Subject: [PATCH 16/49] Have a tiny url state management by using `replace_state` of cable_ready --- app/controllers/admin/products_v3_controller.rb | 5 ++++- app/reflexes/products_reflex.rb | 11 +++++++++++ app/views/admin/products_v3/index.html.haml | 2 +- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index ede851965d..d4fc3f7b4e 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -2,6 +2,9 @@ module Admin class ProductsV3Controller < Spree::Admin::BaseController - def index; end + def index + @page = params[:page] || 1 + @per_page = params[:per_page] || 15 + end end end diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index c3e7a54cb2..2932f772dc 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -28,6 +28,10 @@ class ProductsReflex < ApplicationReflex locals: { products: @products, pagy: @pagy }) ).broadcast + cable_ready.replace_state( + url: current_url, + ).broadcast_later + morph :nothing end @@ -60,4 +64,11 @@ class ProductsReflex < ApplicationReflex # ] ] end + + def current_url + url = URI(request.original_url) + url.query = url.query.present? ? "#{url.query}&" : "" + url.query += "page=#{@page}&per_page=#{@per_page}" + url.to_s + end end diff --git a/app/views/admin/products_v3/index.html.haml b/app/views/admin/products_v3/index.html.haml index 54dd7f363a..d5551ead36 100644 --- a/app/views/admin/products_v3/index.html.haml +++ b/app/views/admin/products_v3/index.html.haml @@ -10,7 +10,7 @@ = render partial: 'spree/admin/shared/product_sub_menu' -#products_v3_page{"data-controller": "products"} +#products_v3_page{"data-controller": "products", "data-page": @page , "data-perpage": @per_page} #loading-spinner.spinner-container{ "data-controller": "loading" } .spinner = t('.loading') From 643897abb27ee61d6033d913975a26c4a18b8371 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Fri, 30 Jun 2023 10:19:26 +0200 Subject: [PATCH 17/49] Add search functionnality --- app/reflexes/products_reflex.rb | 19 ++++++- .../admin/products_v3/_content.html.haml | 3 ++ .../admin/products_v3/_filters.html.haml | 6 +++ .../controllers/products_controller.js | 8 +++ app/webpacker/css/admin/products_v3.scss | 51 ++++++++++++++++--- .../css/admin_v3/globals/palette.scss | 1 + config/locales/en.yml | 3 ++ 7 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 app/views/admin/products_v3/_filters.html.haml diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 2932f772dc..378380ba4c 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -6,6 +6,7 @@ class ProductsReflex < ApplicationReflex def fetch @page ||= element.dataset.page || 1 @per_page ||= element.dataset.perpage || 15 + @search_term ||= element.dataset.searchterm || "" fetch_products @@ -19,13 +20,22 @@ class ProductsReflex < ApplicationReflex fetch end + def filter + @per_page = params[:per_page] + @page = 1 + @search_term = params[:search_term] + + fetch_products + render_products + end + private def render_products cable_ready.replace( selector: "#products-content", html: render(partial: "admin/products_v3/content", - locals: { products: @products, pagy: @pagy }) + locals: { products: @products, pagy: @pagy, search_term: @search_term }) ).broadcast cable_ready.replace_state( @@ -38,7 +48,7 @@ class ProductsReflex < ApplicationReflex # copied from ProductsTableComponent def fetch_products product_query = OpenFoodNetwork::Permissions.new(current_user) - .editable_products.merge(product_scope) + .editable_products.merge(product_scope).ransack(ransack_query).result @pagy, @products = pagy(product_query.order(:name), items: @per_page, page: @page) end @@ -52,6 +62,11 @@ class ProductsReflex < ApplicationReflex scope.includes(product_query_includes) end + def ransack_query + query = { s: "name desc" } + query.merge({ name_cont: @search_term }) if @search_term.present? + end + # Optimise by pre-loading required columns def product_query_includes # TODO: add other fields used in columns? (eg supplier: [:name]) diff --git a/app/views/admin/products_v3/_content.html.haml b/app/views/admin/products_v3/_content.html.haml index 247e51b9a6..157426e009 100644 --- a/app/views/admin/products_v3/_content.html.haml +++ b/app/views/admin/products_v3/_content.html.haml @@ -1,5 +1,8 @@ #products-content - if products.any? + .container + .sixteen.columns + = render partial: 'filters', locals: { search_term: search_term } .container .sixteen.columns = render partial: 'sort', locals: { pagy: pagy } diff --git a/app/views/admin/products_v3/_filters.html.haml b/app/views/admin/products_v3/_filters.html.haml new file mode 100644 index 0000000000..c235cf754f --- /dev/null +++ b/app/views/admin/products_v3/_filters.html.haml @@ -0,0 +1,6 @@ +%form{ id: "filters", 'data-reflex-serialize-form': true, 'data-reflex': 'submit->products#filter' } + .query + .search-input + = text_field_tag :search_term, search_term, placeholder: t('.search_products') + .submit + = submit_tag t(".search"), class: "search-button secondary" diff --git a/app/webpacker/controllers/products_controller.js b/app/webpacker/controllers/products_controller.js index ab65d2a1bf..1d831ce50f 100644 --- a/app/webpacker/controllers/products_controller.js +++ b/app/webpacker/controllers/products_controller.js @@ -15,6 +15,14 @@ export default class extends ApplicationController { this.stimulate("Products#change_per_page", event.target); }; + beforeFilter() { + this.showLoading(); + } + + afterFilter() { + this.hideLoading(); + } + beforeChangePerPage() { this.showLoading(); } diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss index 7eb5a11411..fc59177995 100644 --- a/app/webpacker/css/admin/products_v3.scss +++ b/app/webpacker/css/admin/products_v3.scss @@ -83,17 +83,56 @@ } } - #sort { + #sort, + #filters { margin-bottom: 1em; display: flex; justify-content: space-between; align-items: center; + } - .with-dropdown { - display: flex; - justify-content: space-between; - align-items: center; - gap: 10px; + #sort .with-dropdown { + display: flex; + justify-content: space-between; + align-items: center; + gap: 10px; + } + + #filters { + .query { + flex-grow: 2; + .search-input { + width: 100%; + position: relative; + background-color: $lighter-grey; + border: 1px solid $lighter-grey; + border-radius: 4px; + height: 34px; + line-height: 34px; + + &:has(input:focus), + &:has(input:active) { + border: 1px solid $dark-blue; + } + + > input { + background-color: $lighter-grey; + height: 32px; + line-height: 32px; + } + + &:before { + font-family: FontAwesome; + content: "\f002"; + color: $near-black; + font-size: 16px; + margin-left: 10px; + } + } + } + .submit { + flex-grow: 1; + text-align: right; } } } diff --git a/app/webpacker/css/admin_v3/globals/palette.scss b/app/webpacker/css/admin_v3/globals/palette.scss index a5f0777b9b..92ef7e03e4 100644 --- a/app/webpacker/css/admin_v3/globals/palette.scss +++ b/app/webpacker/css/admin_v3/globals/palette.scss @@ -5,6 +5,7 @@ $teal: #008397 !default; // Teal (Allports) $dark-blue: #004e5b !default; // Dark Blue (Sherpa) $red: #c85136 !default; // Red/Orange (Mojo) $yellow: #ff9300 !default; // Yellow +$lighter-grey: #f8f9fa !default; // Lighter grey $light-grey: #eff1f2 !default; // Light grey $near-black: #191c1d !default; // Near-black $dark-grey: #2e3132 !default; // Dark Grey diff --git a/config/locales/en.yml b/config/locales/en.yml index acc3392d23..416202c4b1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -773,6 +773,9 @@ en: per_page: show: Show per_page: "%{num} per page" + filters: + search_products: Search for products + search: Search content: no_products_found: No products found import_products: Import multiple products From 9d52f0b20ad6fd828e3a377341a27bee56a6e6a4 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Fri, 30 Jun 2023 11:15:05 +0200 Subject: [PATCH 18/49] Loading spinner should be above content (filters, sort, ...) --- app/webpacker/css/admin_v3/components/spinner.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/app/webpacker/css/admin_v3/components/spinner.scss b/app/webpacker/css/admin_v3/components/spinner.scss index 1a965e3c22..d6694b3ed2 100644 --- a/app/webpacker/css/admin_v3/components/spinner.scss +++ b/app/webpacker/css/admin_v3/components/spinner.scss @@ -10,6 +10,7 @@ gap: 40px; font-size: 24px; background: rgba(255, 255, 255, 0.8); + z-index: 2; &.hidden { display: none; From 38d0af8ec0860293c59741e55507d57fcc18aa9d Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Fri, 30 Jun 2023 11:17:42 +0200 Subject: [PATCH 19/49] Simplify: don't need to pass through the js controller Use the reflex itself + Don't need to create a method that will be called only in the connect + Simply code by adding only two lifecycle methods Actually it seems that all reflex related to products controller should show/hide loading --- app/views/admin/products_v3/_sort.html.haml | 2 +- .../controllers/products_controller.js | 30 ++----------------- 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/app/views/admin/products_v3/_sort.html.haml b/app/views/admin/products_v3/_sort.html.haml index c20f696851..04ca72236e 100644 --- a/app/views/admin/products_v3/_sort.html.haml +++ b/app/views/admin/products_v3/_sort.html.haml @@ -3,4 +3,4 @@ = t(".pagination.total_html", total: pagy.count, from: pagy.from, to: pagy.to) %div.with-dropdown = t(".pagination.per_page.show") - = select_tag :per_page, options_for_select([15, 25, 50, 100].collect{|i| [t('.pagination.per_page.per_page', num: i), i]}, pagy.items), data: { action: "change->products#changePerPage" } + = select_tag :per_page, options_for_select([15, 25, 50, 100].collect{|i| [t('.pagination.per_page.per_page', num: i), i]}, pagy.items), data: { reflex: "change->products#change_per_page" } diff --git a/app/webpacker/controllers/products_controller.js b/app/webpacker/controllers/products_controller.js index 1d831ce50f..b79caee14e 100644 --- a/app/webpacker/controllers/products_controller.js +++ b/app/webpacker/controllers/products_controller.js @@ -4,38 +4,14 @@ export default class extends ApplicationController { connect() { super.connect(); // Fetch the products on page load - this.fetch(); - } - - fetch = () => { this.stimulate("Products#fetch"); - }; + } - changePerPage = (event) => { - this.stimulate("Products#change_per_page", event.target); - }; - - beforeFilter() { + beforeReflex() { this.showLoading(); } - afterFilter() { - this.hideLoading(); - } - - beforeChangePerPage() { - this.showLoading(); - } - - afterChangePerPage() { - this.hideLoading(); - } - - beforeFetch() { - this.showLoading(); - } - - afterFetch() { + afterReflex() { this.hideLoading(); } From 51e139b6f1ee73382cacbdbe03a642b90471ca69 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Fri, 30 Jun 2023 15:32:36 +0200 Subject: [PATCH 20/49] Create first spec for the new products page Modify comment, and contextualize search (inside search term) Extract creation of products outside a before block --- .../system/admin/products_v3/products_spec.rb | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 spec/system/admin/products_v3/products_spec.rb diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb new file mode 100644 index 0000000000..0cea950004 --- /dev/null +++ b/spec/system/admin/products_v3/products_spec.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require "system_helper" + +describe 'As an admin, I can see the new product page' do + include WebHelper + include AuthenticationHelper + include FileHelper + + # create lot of products + let!(:products) { create_list(:simple_product, 70) } + # create a product with a name that can be searched + let!(:product_by_name) { create(:simple_product, name: "searchable product") } + before do + # activate feature toggle admin_style_v3 to use new admin interface + Flipper.enable(:admin_style_v3) + login_as_admin + end + + it "can see the new product page" do + visit "/admin/products_v3" + expect(page).to have_content "Bulk Edit Products" + end + + context "pagination" do + before :each do + visit "/admin/products_v3" + end + + it "has a pagination" do + expect(page).to have_selector ".pagination" + end + + it "has 15 products per page by default" do + expect_products_count_to_be 15 + end + + it "can change the page" do + within ".pagination" do + click_link "2" + end + expect_page_to_be 2 + expect_per_page_to_be 15 + expect_products_count_to_be 15 + end + + it "can change the number of products per page" do + select "50", from: "per_page" + expect_page_to_be 1 + expect_per_page_to_be 50 + expect_products_count_to_be 50 + end + end + + context "search" do + before :each do + visit "/admin/products_v3" + end + + context "search by search term" do + it "can search for a product" do + search_for "searchable product" + + expect(page).to have_field "search_term", with: "searchable product" + expect_page_to_be 1 + expect_products_count_to_be 1 + end + + it "reset the page when searching" do + pending "this test is not working" + within ".pagination" do + click_link "2" + end + expect_page_to_be 2 + expect_per_page_to_be 15 + expect_products_count_to_be 15 + search_for "searchable product" + expect_page_to_be 1 + expect_products_count_to_be 1 + end + end + + end + end + end + + def expect_page_to_be(page) + pending "this test is not working" + expect(page).to have_selector ".pagination .page.current", text: page.to_s + end + + def expect_per_page_to_be(per_page) + expect(page).to have_selector "#per_page", text: per_page.to_s + end + + def expect_products_count_to_be(count) + expect(page).to have_selector "table.products tbody", count: count + end + + def search_for(term) + fill_in "search_term", with: term + click_button "Search" + end +end From 762b777995f14053e7f11c196d81dae80d9f4493 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Fri, 30 Jun 2023 15:45:31 +0200 Subject: [PATCH 21/49] Design of the search button --- app/views/admin/products_v3/_filters.html.haml | 3 ++- app/webpacker/css/admin/products_v3.scss | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/app/views/admin/products_v3/_filters.html.haml b/app/views/admin/products_v3/_filters.html.haml index c235cf754f..f7fe523942 100644 --- a/app/views/admin/products_v3/_filters.html.haml +++ b/app/views/admin/products_v3/_filters.html.haml @@ -3,4 +3,5 @@ .search-input = text_field_tag :search_term, search_term, placeholder: t('.search_products') .submit - = submit_tag t(".search"), class: "search-button secondary" + .search-button + = submit_tag t(".search"), class: "secondary" diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss index fc59177995..2a3d397926 100644 --- a/app/webpacker/css/admin/products_v3.scss +++ b/app/webpacker/css/admin/products_v3.scss @@ -133,6 +133,22 @@ .submit { flex-grow: 1; text-align: right; + + .search-button { + position: relative; + > input { + padding-left: 30px; + } + &:before { + position: absolute; + font-family: FontAwesome; + content: "\f002"; + font-size: 16px; + margin-left: 10px; + line-height: 32px; + color: $teal; + } + } } } } From ddfc60c85e100861bf0ca1f21506c0f69ea0957c Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Fri, 30 Jun 2023 15:53:51 +0200 Subject: [PATCH 22/49] Don't use events, but call the loading controller itself --- app/views/admin/products_v3/index.html.haml | 2 +- .../controllers/loading_controller.js | 7 ------- .../controllers/products_controller.js | 19 +++++++++++++++---- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/app/views/admin/products_v3/index.html.haml b/app/views/admin/products_v3/index.html.haml index d5551ead36..26e10e7559 100644 --- a/app/views/admin/products_v3/index.html.haml +++ b/app/views/admin/products_v3/index.html.haml @@ -11,7 +11,7 @@ = render partial: 'spree/admin/shared/product_sub_menu' #products_v3_page{"data-controller": "products", "data-page": @page , "data-perpage": @per_page} - #loading-spinner.spinner-container{ "data-controller": "loading" } + #loading-spinner.spinner-container{ "data-controller": "loading", "data-products-target": "loading" } .spinner = t('.loading') #products-content diff --git a/app/webpacker/controllers/loading_controller.js b/app/webpacker/controllers/loading_controller.js index 903cc9898d..4d93c2bd6c 100644 --- a/app/webpacker/controllers/loading_controller.js +++ b/app/webpacker/controllers/loading_controller.js @@ -3,13 +3,6 @@ import ApplicationController from "./application_controller"; export default class extends ApplicationController { connect() { super.connect(); - document.addEventListener("show-loading", this.showLoading); - document.addEventListener("hide-loading", this.hideLoading); - } - - disconnect() { - document.removeEventListener("show-loading", this.showLoading); - document.removeEventListener("hide-loading", this.hideLoading); } hideLoading = () => { diff --git a/app/webpacker/controllers/products_controller.js b/app/webpacker/controllers/products_controller.js index b79caee14e..ed8d5e02c9 100644 --- a/app/webpacker/controllers/products_controller.js +++ b/app/webpacker/controllers/products_controller.js @@ -1,6 +1,8 @@ import ApplicationController from "./application_controller"; export default class extends ApplicationController { + static targets = ["loading"]; + connect() { super.connect(); // Fetch the products on page load @@ -16,12 +18,21 @@ export default class extends ApplicationController { } showLoading = () => { - const event = new CustomEvent("show-loading"); - document.dispatchEvent(event); + if (this.getLoadingController()) { + this.getLoadingController().showLoading(); + } }; hideLoading = () => { - const event = new CustomEvent("hide-loading"); - document.dispatchEvent(event); + if (this.getLoadingController()) { + this.getLoadingController().hideLoading(); + } + }; + + getLoadingController = () => { + return (this.loadongController = this.application.getControllerForElementAndIdentifier( + this.loadingTarget, + "loading" + )); }; } From f58cf2d3b2126e2d12c6978d619cfdde211e42db Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 3 Jul 2023 12:31:36 +0200 Subject: [PATCH 23/49] Can filter by producer Not sur the request nor the `producers` in the reflex should be like this. This is a proof of concept, and should probably be reviewed Fix linter issues --- app/reflexes/products_reflex.rb | 17 +++++++++++-- .../admin/products_v3/_content.html.haml | 2 +- .../admin/products_v3/_filters.html.haml | 2 ++ app/webpacker/css/admin/products_v3.scss | 1 + config/locales/en.yml | 1 + .../system/admin/products_v3/products_spec.rb | 24 ++++++++++++++++++- 6 files changed, 43 insertions(+), 4 deletions(-) diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 378380ba4c..0ab4035927 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -24,6 +24,7 @@ class ProductsReflex < ApplicationReflex @per_page = params[:per_page] @page = 1 @search_term = params[:search_term] + @producer_id = params[:producer_id] fetch_products render_products @@ -35,7 +36,8 @@ class ProductsReflex < ApplicationReflex cable_ready.replace( selector: "#products-content", html: render(partial: "admin/products_v3/content", - locals: { products: @products, pagy: @pagy, search_term: @search_term }) + locals: { products: @products, pagy: @pagy, search_term: @search_term, + producer_options: producers, producer_id: @producer_id }) ).broadcast cable_ready.replace_state( @@ -45,6 +47,15 @@ class ProductsReflex < ApplicationReflex morph :nothing end + def producers + producers = if current_user.has_spree_role?("admin") + Enterprise.all + else + current_user.enterprises + end + producers.map { |p| [p.name, p.id] } + end + # copied from ProductsTableComponent def fetch_products product_query = OpenFoodNetwork::Permissions.new(current_user) @@ -64,7 +75,9 @@ class ProductsReflex < ApplicationReflex def ransack_query query = { s: "name desc" } - query.merge({ name_cont: @search_term }) if @search_term.present? + query = query.merge({ supplier_id_in: @producer_id }) if @producer_id.present? + query = query.merge({ name_cont: @search_term }) if @search_term.present? + query end # Optimise by pre-loading required columns diff --git a/app/views/admin/products_v3/_content.html.haml b/app/views/admin/products_v3/_content.html.haml index 157426e009..e7e6e9ee3a 100644 --- a/app/views/admin/products_v3/_content.html.haml +++ b/app/views/admin/products_v3/_content.html.haml @@ -2,7 +2,7 @@ - if products.any? .container .sixteen.columns - = render partial: 'filters', locals: { search_term: search_term } + = render partial: 'filters', locals: { search_term: search_term, producer_id: producer_id, producer_options: producer_options } .container .sixteen.columns = render partial: 'sort', locals: { pagy: pagy } diff --git a/app/views/admin/products_v3/_filters.html.haml b/app/views/admin/products_v3/_filters.html.haml index f7fe523942..26a1d9e3c6 100644 --- a/app/views/admin/products_v3/_filters.html.haml +++ b/app/views/admin/products_v3/_filters.html.haml @@ -2,6 +2,8 @@ .query .search-input = text_field_tag :search_term, search_term, placeholder: t('.search_products') + .producers + = select_tag :producer_id, options_for_select(producer_options, producer_id), include_blank: t('.all_producers') .submit .search-button = submit_tag t(".search"), class: "secondary" diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss index 2a3d397926..fda1510109 100644 --- a/app/webpacker/css/admin/products_v3.scss +++ b/app/webpacker/css/admin/products_v3.scss @@ -99,6 +99,7 @@ } #filters { + gap: 20px; .query { flex-grow: 2; .search-input { diff --git a/config/locales/en.yml b/config/locales/en.yml index 416202c4b1..bca84fb4b2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -775,6 +775,7 @@ en: per_page: "%{num} per page" filters: search_products: Search for products + all_producers: All producers search: Search content: no_products_found: No products found diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index 0cea950004..fe809fe6ed 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -11,6 +11,12 @@ describe 'As an admin, I can see the new product page' do let!(:products) { create_list(:simple_product, 70) } # create a product with a name that can be searched let!(:product_by_name) { create(:simple_product, name: "searchable product") } + # create a product with a supplier that can be searched + let!(:product_by_supplier) { + create(:simple_product, + supplier: create(:enterprise, name: "Producer 1")) + } + before do # activate feature toggle admin_style_v3 to use new admin interface Flipper.enable(:admin_style_v3) @@ -80,6 +86,17 @@ describe 'As an admin, I can see the new product page' do end end + context "search by producer" do + it "has a producer select" do + expect(page).to have_selector "select#producer_id" + end + + it "can search for a product" do + search_by_producer "Producer 1" + + expect(page).to have_select "producer_id", selected: "Producer 1" + expect_page_to_be 1 + expect_products_count_to_be 1 end end end @@ -94,11 +111,16 @@ describe 'As an admin, I can see the new product page' do end def expect_products_count_to_be(count) - expect(page).to have_selector "table.products tbody", count: count + expect(page).to have_selector("table.products tbody", count:) end def search_for(term) fill_in "search_term", with: term click_button "Search" end + + def search_by_producer(producer) + select producer, from: "producer_id" + click_button "Search" + end end From ef1702188f06ab210ef541ff015261e284349b7a Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 3 Jul 2023 13:35:14 +0200 Subject: [PATCH 24/49] Extract filters from 'no products found' logic We still want to display filters if no products found --- app/views/admin/products_v3/_content.html.haml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/views/admin/products_v3/_content.html.haml b/app/views/admin/products_v3/_content.html.haml index e7e6e9ee3a..24f0150424 100644 --- a/app/views/admin/products_v3/_content.html.haml +++ b/app/views/admin/products_v3/_content.html.haml @@ -1,8 +1,10 @@ #products-content + .container + .sixteen.columns + = render partial: 'filters', locals: { search_term: search_term, + producer_id: producer_id, + producer_options: producer_options } - if products.any? - .container - .sixteen.columns - = render partial: 'filters', locals: { search_term: search_term, producer_id: producer_id, producer_options: producer_options } .container .sixteen.columns = render partial: 'sort', locals: { pagy: pagy } From bfe1884ab51ae83cde6756718609cdc479d3a7ff Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 3 Jul 2023 13:37:35 +0200 Subject: [PATCH 25/49] Can filter by categories --- app/reflexes/products_reflex.rb | 9 +++++++- .../admin/products_v3/_content.html.haml | 4 +++- .../admin/products_v3/_filters.html.haml | 2 ++ config/locales/en.yml | 1 + .../system/admin/products_v3/products_spec.rb | 23 +++++++++++++++++++ 5 files changed, 37 insertions(+), 2 deletions(-) diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 0ab4035927..3a3047513d 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -25,6 +25,7 @@ class ProductsReflex < ApplicationReflex @page = 1 @search_term = params[:search_term] @producer_id = params[:producer_id] + @category_id = params[:category_id] fetch_products render_products @@ -37,7 +38,8 @@ class ProductsReflex < ApplicationReflex selector: "#products-content", html: render(partial: "admin/products_v3/content", locals: { products: @products, pagy: @pagy, search_term: @search_term, - producer_options: producers, producer_id: @producer_id }) + producer_options: producers, producer_id: @producer_id, + category_options: categories, category_id: @category_id }) ).broadcast cable_ready.replace_state( @@ -56,6 +58,10 @@ class ProductsReflex < ApplicationReflex producers.map { |p| [p.name, p.id] } end + def categories + Spree::Taxon.order(:name).map { |c| [c.name, c.id] } + end + # copied from ProductsTableComponent def fetch_products product_query = OpenFoodNetwork::Permissions.new(current_user) @@ -77,6 +83,7 @@ class ProductsReflex < ApplicationReflex query = { s: "name desc" } query = query.merge({ supplier_id_in: @producer_id }) if @producer_id.present? query = query.merge({ name_cont: @search_term }) if @search_term.present? + query = query.merge({ primary_taxon_id_in: @category_id }) if @category_id.present? query end diff --git a/app/views/admin/products_v3/_content.html.haml b/app/views/admin/products_v3/_content.html.haml index 24f0150424..e9fcd26975 100644 --- a/app/views/admin/products_v3/_content.html.haml +++ b/app/views/admin/products_v3/_content.html.haml @@ -3,7 +3,9 @@ .sixteen.columns = render partial: 'filters', locals: { search_term: search_term, producer_id: producer_id, - producer_options: producer_options } + producer_options: producer_options, + category_options: category_options, + category_id: category_id } - if products.any? .container .sixteen.columns diff --git a/app/views/admin/products_v3/_filters.html.haml b/app/views/admin/products_v3/_filters.html.haml index 26a1d9e3c6..fca9e951e6 100644 --- a/app/views/admin/products_v3/_filters.html.haml +++ b/app/views/admin/products_v3/_filters.html.haml @@ -4,6 +4,8 @@ = text_field_tag :search_term, search_term, placeholder: t('.search_products') .producers = select_tag :producer_id, options_for_select(producer_options, producer_id), include_blank: t('.all_producers') + .categories + = select_tag :category_id, options_for_select(category_options, category_id), include_blank: t('.all_categories') .submit .search-button = submit_tag t(".search"), class: "secondary" diff --git a/config/locales/en.yml b/config/locales/en.yml index bca84fb4b2..d9b9113a82 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -776,6 +776,7 @@ en: filters: search_products: Search for products all_producers: All producers + all_categories: All categories search: Search content: no_products_found: No products found diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index fe809fe6ed..a0f6242bfb 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -16,6 +16,10 @@ describe 'As an admin, I can see the new product page' do create(:simple_product, supplier: create(:enterprise, name: "Producer 1")) } + # create a product with a category that can be searched + let!(:product_by_category) { + create(:simple_product, taxons: [create(:taxon, name: "Category 1")]) + } before do # activate feature toggle admin_style_v3 to use new admin interface @@ -99,6 +103,20 @@ describe 'As an admin, I can see the new product page' do expect_products_count_to_be 1 end end + + context "search by category" do + it "has a category select" do + expect(page).to have_selector "select#category_id" + end + + it "can search for a product" do + search_by_category "Category 1" + + expect(page).to have_select "category_id", selected: "Category 1" + expect_page_to_be 1 + expect_products_count_to_be 1 + end + end end def expect_page_to_be(page) @@ -123,4 +141,9 @@ describe 'As an admin, I can see the new product page' do select producer, from: "producer_id" click_button "Search" end + + def search_by_category(category) + select category, from: "category_id" + click_button "Search" + end end From 333dc11fc1d2d41be67725ec49705802c48b015b Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 3 Jul 2023 15:01:33 +0200 Subject: [PATCH 26/49] Manage form params vs. URL params. And replace state with params. "One needs to understand what the source of the params in a reflex is. And there are two sources: the closest form the url of the currently displayed page" Source: https://github.com/stimulusreflex/stimulus_reflex/issues/290#issuecomment-683334963 --- .../admin/products_v3_controller.rb | 5 +- app/reflexes/products_reflex.rb | 51 +++++++++++++------ app/views/admin/products_v3/index.html.haml | 2 +- 3 files changed, 38 insertions(+), 20 deletions(-) diff --git a/app/controllers/admin/products_v3_controller.rb b/app/controllers/admin/products_v3_controller.rb index d4fc3f7b4e..ede851965d 100644 --- a/app/controllers/admin/products_v3_controller.rb +++ b/app/controllers/admin/products_v3_controller.rb @@ -2,9 +2,6 @@ module Admin class ProductsV3Controller < Spree::Admin::BaseController - def index - @page = params[:page] || 1 - @per_page = params[:per_page] || 15 - end + def index; end end end diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 3a3047513d..94159bbf8c 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -3,36 +3,52 @@ class ProductsReflex < ApplicationReflex include Pagy::Backend + before_reflex :init_params + def fetch - @page ||= element.dataset.page || 1 - @per_page ||= element.dataset.perpage || 15 - @search_term ||= element.dataset.searchterm || "" - - fetch_products - - render_products + fetch_and_render_products end def change_per_page @per_page = element.value.to_i @page = 1 - fetch + fetch_and_render_products end def filter - @per_page = params[:per_page] @page = 1 - @search_term = params[:search_term] - @producer_id = params[:producer_id] - @category_id = params[:category_id] - fetch_products - render_products + fetch_and_render_products end private + def init_params + init_filters_params + init_pagination_params + end + + def init_filters_params + # params comes from the form + # _params comes from the url + # priority is given to params from the form (if present) over url params + @search_term = params[:search_term] || params[:_search_term] + @producer_id = params[:producer_id] || params[:_producer_id] + @category_id = params[:category_id] || params[:_category_id] + end + + def init_pagination_params + # prority is given to element dataset (if present) over url params + @page = element.dataset.page || params[:_page] || 1 + @per_page = element.dataset.perpage || params[:_per_page] || 15 + end + + def fetch_and_render_products + fetch_products + render_products + end + def render_products cable_ready.replace( selector: "#products-content", @@ -103,7 +119,12 @@ class ProductsReflex < ApplicationReflex def current_url url = URI(request.original_url) url.query = url.query.present? ? "#{url.query}&" : "" - url.query += "page=#{@page}&per_page=#{@per_page}" + # add params with _ to avoid conflicts with params from the form + url.query += "_page=#{@page}" + url.query += "&_per_page=#{@per_page}" + url.query += "&_search_term=#{@search_term}" if @search_term.present? + url.query += "&_producer_id=#{@producer_id}" if @producer_id.present? + url.query += "&_category_id=#{@category_id}" if @category_id.present? url.to_s end end diff --git a/app/views/admin/products_v3/index.html.haml b/app/views/admin/products_v3/index.html.haml index 26e10e7559..581dcc2605 100644 --- a/app/views/admin/products_v3/index.html.haml +++ b/app/views/admin/products_v3/index.html.haml @@ -10,7 +10,7 @@ = render partial: 'spree/admin/shared/product_sub_menu' -#products_v3_page{"data-controller": "products", "data-page": @page , "data-perpage": @per_page} +#products_v3_page{ "data-controller": "products" } #loading-spinner.spinner-container{ "data-controller": "loading", "data-products-target": "loading" } .spinner = t('.loading') From 7b56cbf3d49e0f5ad3a4d9c8d5d0ab69ad50b218 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 3 Jul 2023 15:15:08 +0200 Subject: [PATCH 27/49] Add labels to selectors, and adjust css + Adjusting filter elements with flex-grow properties --- .../admin/products_v3/_filters.html.haml | 2 ++ app/webpacker/css/admin/products_v3.scss | 33 +++++++++++++++++-- config/locales/en.yml | 4 +++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/app/views/admin/products_v3/_filters.html.haml b/app/views/admin/products_v3/_filters.html.haml index fca9e951e6..02ccc9f016 100644 --- a/app/views/admin/products_v3/_filters.html.haml +++ b/app/views/admin/products_v3/_filters.html.haml @@ -3,8 +3,10 @@ .search-input = text_field_tag :search_term, search_term, placeholder: t('.search_products') .producers + .label= t('.producers.label') = select_tag :producer_id, options_for_select(producer_options, producer_id), include_blank: t('.all_producers') .categories + .label= t('.categories.label') = select_tag :category_id, options_for_select(category_options, category_id), include_blank: t('.all_categories') .submit .search-button diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss index fda1510109..311bc3ee72 100644 --- a/app/webpacker/css/admin/products_v3.scss +++ b/app/webpacker/css/admin/products_v3.scss @@ -100,8 +100,30 @@ #filters { gap: 20px; + align-items: flex-end; + + .producers, + .categories { + > .label { + margin-left: 3px; + margin-bottom: 2px; + } + } + + .query { + flex-grow: 1; + } + + .producers, + .categories { + flex-grow: 0; + } + + .submit { + flex-grow: 0; + } + .query { - flex-grow: 2; .search-input { width: 100%; position: relative; @@ -131,8 +153,15 @@ } } } + + .producers, + .categories { + select { + width: 150px; + } + } + .submit { - flex-grow: 1; text-align: right; .search-button { diff --git a/config/locales/en.yml b/config/locales/en.yml index d9b9113a82..0c8131dc36 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -777,6 +777,10 @@ en: search_products: Search for products all_producers: All producers all_categories: All categories + producers: + label: Producers + categories: + label: Categories search: Search content: no_products_found: No products found From 904c7bfacf15c2505b29a15827037ce10728942a Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 3 Jul 2023 16:21:11 +0200 Subject: [PATCH 28/49] Buttons design Remove already defined `$border-radius` Defined `app/webpacker/css/admin/globals/variables.scss` Adjust height --- .../admin/products_v3/_filters.html.haml | 2 +- app/webpacker/css/admin/products_v3.scss | 16 +--- app/webpacker/css/admin/variables.scss | 2 - app/webpacker/css/admin_v3/all.scss | 3 +- .../css/admin_v3/components/buttons.scss | 79 +++++++++++++++++++ .../css/admin_v3/globals/palette.scss | 4 + .../css/admin_v3/globals/variables.scss | 7 +- app/webpacker/css/admin_v3/shared/icons.scss | 42 ++++++++++ 8 files changed, 136 insertions(+), 19 deletions(-) create mode 100644 app/webpacker/css/admin_v3/components/buttons.scss create mode 100644 app/webpacker/css/admin_v3/shared/icons.scss diff --git a/app/views/admin/products_v3/_filters.html.haml b/app/views/admin/products_v3/_filters.html.haml index 02ccc9f016..7d0c9b3b28 100644 --- a/app/views/admin/products_v3/_filters.html.haml +++ b/app/views/admin/products_v3/_filters.html.haml @@ -10,4 +10,4 @@ = select_tag :category_id, options_for_select(category_options, category_id), include_blank: t('.all_categories') .submit .search-button - = submit_tag t(".search"), class: "secondary" + = button_tag t(".search"), class: "secondary icon-search" diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss index 311bc3ee72..e7bd48858d 100644 --- a/app/webpacker/css/admin/products_v3.scss +++ b/app/webpacker/css/admin/products_v3.scss @@ -130,8 +130,8 @@ background-color: $lighter-grey; border: 1px solid $lighter-grey; border-radius: 4px; - height: 34px; - line-height: 34px; + height: $btn-height; + line-height: $btn-height; &:has(input:focus), &:has(input:active) { @@ -140,8 +140,6 @@ > input { background-color: $lighter-grey; - height: 32px; - line-height: 32px; } &:before { @@ -158,6 +156,7 @@ .categories { select { width: 150px; + height: $btn-height; } } @@ -169,15 +168,6 @@ > input { padding-left: 30px; } - &:before { - position: absolute; - font-family: FontAwesome; - content: "\f002"; - font-size: 16px; - margin-left: 10px; - line-height: 32px; - color: $teal; - } } } } diff --git a/app/webpacker/css/admin/variables.scss b/app/webpacker/css/admin/variables.scss index f93b93a57f..19221dc04e 100644 --- a/app/webpacker/css/admin/variables.scss +++ b/app/webpacker/css/admin/variables.scss @@ -16,5 +16,3 @@ $admin-table-border: $pale-blue; $modal-close-button-color: #de6060; $modal-close-button-hover-color: #bf4545; $disabled-button: $light-grey; - -$border-radius: 3px; diff --git a/app/webpacker/css/admin_v3/all.scss b/app/webpacker/css/admin_v3/all.scss index 0655e5773b..7b666a4f6d 100644 --- a/app/webpacker/css/admin_v3/all.scss +++ b/app/webpacker/css/admin_v3/all.scss @@ -31,7 +31,7 @@ @import "../shared/utilities"; @import "../admin/shared/typography"; @import "shared/tables"; // admin_v3 -@import "../admin/shared/icons"; +@import "shared/icons"; // admin_v3 @import "../admin/shared/forms"; @import "shared/layout"; // admin_v3 @import "../admin/shared/scroll_bar"; @@ -52,6 +52,7 @@ @import "../admin/components/alert-box"; @import "../admin/components/alert_row"; @import "../admin/components/buttons"; +@import "components/buttons"; // admin_v3 @import "../admin/components/date-picker"; @import "../admin/components/dialogs"; @import "../admin/components/input"; diff --git a/app/webpacker/css/admin_v3/components/buttons.scss b/app/webpacker/css/admin_v3/components/buttons.scss new file mode 100644 index 0000000000..270fcb8f0a --- /dev/null +++ b/app/webpacker/css/admin_v3/components/buttons.scss @@ -0,0 +1,79 @@ +input[type="submit"], +input[type="button"]:not(.trix-button), +button:not(.plain):not(.trix-button), +.button { + position: relative; + cursor: pointer; + font-size: 14px; + @include border-radius($border-radius); + display: inline-block; + padding: 0px 12px; + background-color: $color-btn-bg; + border: 1px solid $color-btn-bg; + color: $color-btn-text; + text-transform: uppercase; + line-height: 40px; + height: 40px; + + &:before { + font-weight: normal !important; + } + + &:active, + &:focus { + outline: none; + border: 1px solid $color-btn-hover-border; + } + + &:hover { + background-color: $color-btn-hover-bg; + border: 1px solid $color-btn-hover-bg; + color: $color-btn-hover-text; + } + + &.fullwidth { + width: 100%; + text-align: center; + } + + &.secondary { + background-color: transparent; + border: 1px solid $color-btn-bg; + color: $color-btn-bg; + + &:hover { + background-color: $color-11; + border: 1px solid $color-10; + color: $color-10; + } + + &:active, + &:focus { + background-color: $color-11; + border: 1px solid $color-4; + color: $color-4; + } + } + + .badge { + position: absolute; + top: 0; + right: 0; + transform: translateY(-50%); + font-size: 10px; + text-transform: capitalize; + padding: 0px 5px; + border-radius: 3px; + + &:before { + padding: 0; + } + + &.danger { + background-color: $warning-red; + } + &.success { + background-color: $spree-green; + } + } +} diff --git a/app/webpacker/css/admin_v3/globals/palette.scss b/app/webpacker/css/admin_v3/globals/palette.scss index 92ef7e03e4..8dc52e1d86 100644 --- a/app/webpacker/css/admin_v3/globals/palette.scss +++ b/app/webpacker/css/admin_v3/globals/palette.scss @@ -2,9 +2,11 @@ $white: #ffffff !default; // White $green: #9fc820 !default; // Green $teal: #008397 !default; // Teal (Allports) +$orient: #006878 !default; // Orient (Cerulean) $dark-blue: #004e5b !default; // Dark Blue (Sherpa) $red: #c85136 !default; // Red/Orange (Mojo) $yellow: #ff9300 !default; // Yellow +$mystic: #d9e8eb !default; // Mystic $lighter-grey: #f8f9fa !default; // Lighter grey $light-grey: #eff1f2 !default; // Light grey $near-black: #191c1d !default; // Near-black @@ -20,3 +22,5 @@ $color-6: $yellow; $color-7: $light-grey; $color-8: $near-black; $color-9: $dark-grey; +$color-10: $orient; +$color-11: $mystic; diff --git a/app/webpacker/css/admin_v3/globals/variables.scss b/app/webpacker/css/admin_v3/globals/variables.scss index 434042f31f..1efa62199a 100644 --- a/app/webpacker/css/admin_v3/globals/variables.scss +++ b/app/webpacker/css/admin_v3/globals/variables.scss @@ -39,8 +39,9 @@ $padding-tbl-cell-relaxed: 16px 12px; $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: lighten($color-btn-bg, 2) !default; +$color-btn-hover-bg: $orient !default; $color-btn-hover-text: $white !default; +$color-btn-hover-border: $dark-blue !default; // Actions colors $color-action-edit-bg: very-light($color-success, 5 ) !default; @@ -142,7 +143,9 @@ $h3-size: $h4-size + 2 !default; $h2-size: $h3-size + 2 !default; $h1-size: $h2-size + 2 !default; -$border-radius: 3px !default; +$border-radius: 4px !default; $font-weight-bold: 600 !default; $font-weight-normal: 400 !default; + +$btn-height: 40px !default; diff --git a/app/webpacker/css/admin_v3/shared/icons.scss b/app/webpacker/css/admin_v3/shared/icons.scss new file mode 100644 index 0000000000..0bc8f0341d --- /dev/null +++ b/app/webpacker/css/admin_v3/shared/icons.scss @@ -0,0 +1,42 @@ +// Some fixes for fontwesome stylesheets +[class*="icon-"] { + &:before { + padding-right: 5px; + } + + &.button, + &.icon_link { + &:before { + padding-right: 8px; + } + } +} + +// for the button tagname as well +button[class*="icon-"] { + &:before { + padding-right: 8px; + } +} + +.icon-email:before { + @extend .icon-envelope, :before; +} +.icon-resend_authorization_email:before { + @extend .icon-envelope, :before; +} +.icon-resume:before { + @extend .icon-refresh, :before; +} + +.icon-cancel:before, +.icon-void:before { + @extend .icon-remove, :before; +} + +.icon-capture { + @extend .icon-ok; +} +.icon-credit:before { + @extend .icon-ok, :before; +} From 5a8a187f546a4cb6cfc0a28206ef80212d47b3f2 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 5 Jul 2023 11:03:50 +0200 Subject: [PATCH 29/49] Add clear search button and action --- app/reflexes/products_reflex.rb | 9 ++++++++ .../admin/products_v3/_content.html.haml | 2 +- app/views/admin/products_v3/_sort.html.haml | 3 +++ .../css/admin_v3/components/buttons.scss | 23 +++++++++++++++++++ .../css/admin_v3/globals/palette.scss | 4 ++++ config/locales/en.yml | 1 + .../system/admin/products_v3/products_spec.rb | 14 +++++++++++ 7 files changed, 55 insertions(+), 1 deletion(-) diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 94159bbf8c..537a7b9d3b 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -22,6 +22,15 @@ class ProductsReflex < ApplicationReflex fetch_and_render_products end + def clear_search + @search_term = nil + @producer_id = nil + @category_id = nil + @page = 1 + + fetch_and_render_products + end + private def init_params diff --git a/app/views/admin/products_v3/_content.html.haml b/app/views/admin/products_v3/_content.html.haml index e9fcd26975..beb3942621 100644 --- a/app/views/admin/products_v3/_content.html.haml +++ b/app/views/admin/products_v3/_content.html.haml @@ -9,7 +9,7 @@ - if products.any? .container .sixteen.columns - = render partial: 'sort', locals: { pagy: pagy } + = render partial: 'sort', locals: { pagy: pagy, search_term: search_term, producer_id: producer_id, category_id: category_id } = render partial: 'table', locals: { products: products } = render partial: 'admin/shared/v3/pagy', locals: { pagy: pagy, reflex: "click->Products#fetch" } - else diff --git a/app/views/admin/products_v3/_sort.html.haml b/app/views/admin/products_v3/_sort.html.haml index 04ca72236e..20c67e083b 100644 --- a/app/views/admin/products_v3/_sort.html.haml +++ b/app/views/admin/products_v3/_sort.html.haml @@ -1,6 +1,9 @@ #sort %div = t(".pagination.total_html", total: pagy.count, from: pagy.from, to: pagy.to) + - if search_term.present? || producer_id.present? || category_id.present? + %a{ href: "#", class: "button disruptive", data: { reflex: "click->products#clear_search" } } + = t(".pagination.clear_search") %div.with-dropdown = t(".pagination.per_page.show") = select_tag :per_page, options_for_select([15, 25, 50, 100].collect{|i| [t('.pagination.per_page.per_page', num: i), i]}, pagy.items), data: { reflex: "change->products#change_per_page" } diff --git a/app/webpacker/css/admin_v3/components/buttons.scss b/app/webpacker/css/admin_v3/components/buttons.scss index 270fcb8f0a..5ad38c345a 100644 --- a/app/webpacker/css/admin_v3/components/buttons.scss +++ b/app/webpacker/css/admin_v3/components/buttons.scss @@ -25,6 +25,10 @@ button:not(.plain):not(.trix-button), border: 1px solid $color-btn-hover-border; } + &:active:focus { + box-shadow: none; + } + &:hover { background-color: $color-btn-hover-bg; border: 1px solid $color-btn-hover-bg; @@ -55,6 +59,25 @@ button:not(.plain):not(.trix-button), } } + &.disruptive { + background-color: transparent; + border: 1px solid $color-5; + color: $color-5; + + &:hover { + background-color: $fair-pink; + border: 1px solid $color-5; + color: $color-5; + } + + &:active, + &:focus { + background-color: $fair-pink; + border: 1px solid $roof-terracotta; + color: $roof-terracotta; + } + } + .badge { position: absolute; top: 0; diff --git a/app/webpacker/css/admin_v3/globals/palette.scss b/app/webpacker/css/admin_v3/globals/palette.scss index 8dc52e1d86..560e1074ac 100644 --- a/app/webpacker/css/admin_v3/globals/palette.scss +++ b/app/webpacker/css/admin_v3/globals/palette.scss @@ -11,6 +11,8 @@ $lighter-grey: #f8f9fa !default; // Lighter grey $light-grey: #eff1f2 !default; // Light grey $near-black: #191c1d !default; // Near-black $dark-grey: #2e3132 !default; // Dark Grey +$fair-pink: #ffefeb !default; // Fair Pink +$roof-terracotta: #b83b1f !default; // Roof Terracotta // Old colour variables for backwards compatibility $color-1: $white; @@ -24,3 +26,5 @@ $color-8: $near-black; $color-9: $dark-grey; $color-10: $orient; $color-11: $mystic; +$color-12: $fair-pink; +$color-13: $roof-terracotta; diff --git a/config/locales/en.yml b/config/locales/en.yml index 0c8131dc36..7c27aea2c6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -773,6 +773,7 @@ en: per_page: show: Show per_page: "%{num} per page" + clear_search: Clear search filters: search_products: Search for products all_producers: All producers diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index a0f6242bfb..6b8dbfb592 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -117,6 +117,20 @@ describe 'As an admin, I can see the new product page' do expect_products_count_to_be 1 end end + + context "clear filters" do + it "can clear filters" do + search_for "searchable product" + expect(page).to have_field "search_term", with: "searchable product" + expect_page_to_be 1 + expect_products_count_to_be 1 + + click_link "Clear search" + expect(page).to have_field "search_term", with: "" + expect_page_to_be 1 + expect_products_count_to_be 15 + end + end end def expect_page_to_be(page) From c378ad1d88ec243cc9050bc5c76568195e2805e2 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 5 Jul 2023 11:50:13 +0200 Subject: [PATCH 30/49] Add "no results found" container --- app/views/admin/products_v3/_content.html.haml | 7 +------ app/views/admin/products_v3/_no_products.html.haml | 11 +++++++++++ config/locales/en.yml | 2 ++ spec/system/admin/products_v3/products_spec.rb | 8 ++++++++ 4 files changed, 22 insertions(+), 6 deletions(-) create mode 100644 app/views/admin/products_v3/_no_products.html.haml diff --git a/app/views/admin/products_v3/_content.html.haml b/app/views/admin/products_v3/_content.html.haml index beb3942621..b598120c14 100644 --- a/app/views/admin/products_v3/_content.html.haml +++ b/app/views/admin/products_v3/_content.html.haml @@ -14,9 +14,4 @@ = render partial: 'admin/shared/v3/pagy', locals: { pagy: pagy, reflex: "click->Products#fetch" } - else #no-products - = t('.no_products_found') - #no-products-actions - %a{ href: "/admin/products/new", class: "button icon-plus", icon: "icon-plus" } - = t(:new_product) - %a{ href: "/admin/products/import", class: "button icon-upload secondary", icon: "icon-upload" } - = t(".import_products") + = render partial: "no_products", locals: { search_term: search_term, producer_id: producer_id, category_id: category_id } diff --git a/app/views/admin/products_v3/_no_products.html.haml b/app/views/admin/products_v3/_no_products.html.haml new file mode 100644 index 0000000000..7b0bbb328c --- /dev/null +++ b/app/views/admin/products_v3/_no_products.html.haml @@ -0,0 +1,11 @@ +- if search_term.present? || producer_id.present? || category_id.present? + = t('.no_products_found_for_search') + %a{ href: "#", class: "button disruptive", data: { reflex: "click->products#clear_search" } } + = t("admin.products_v3.sort.pagination.clear_search") +- else + = t('.no_products_found') + #no-products-actions + %a{ href: "/admin/products/new", class: "button icon-plus", icon: "icon-plus" } + = t(:new_product) + %a{ href: "/admin/products/import", class: "button icon-upload secondary", icon: "icon-upload" } + = t(".import_products") diff --git a/config/locales/en.yml b/config/locales/en.yml index 7c27aea2c6..682e090f86 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -784,8 +784,10 @@ en: label: Categories search: Search content: + no_products: no_products_found: No products found import_products: Import multiple products + no_products_found_for_search: No products found for your search criteria product_import: title: Product Import file_not_found: File not found or could not be opened diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index 6b8dbfb592..f7b521ec27 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -131,6 +131,14 @@ describe 'As an admin, I can see the new product page' do expect_products_count_to_be 15 end end + + context "no results" do + it "shows a message when there are no results" do + search_for "no results" + expect(page).to have_content "No products found for your search criteria" + expect(page).to have_link "Clear search" + end + end end def expect_page_to_be(page) From 010f19cb83f760c6cec2769d1938a506cfe564d4 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 5 Jul 2023 14:02:15 +0200 Subject: [PATCH 31/49] Create a medium height button, and adjust `#sort` row --- app/views/admin/products_v3/_sort.html.haml | 2 +- app/webpacker/css/admin/products_v3.scss | 15 ++++++++++----- .../css/admin_v3/components/buttons.scss | 5 +++++ app/webpacker/css/admin_v3/globals/variables.scss | 1 + 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/app/views/admin/products_v3/_sort.html.haml b/app/views/admin/products_v3/_sort.html.haml index 20c67e083b..d3daa5eaa3 100644 --- a/app/views/admin/products_v3/_sort.html.haml +++ b/app/views/admin/products_v3/_sort.html.haml @@ -2,7 +2,7 @@ %div = t(".pagination.total_html", total: pagy.count, from: pagy.from, to: pagy.to) - if search_term.present? || producer_id.present? || category_id.present? - %a{ href: "#", class: "button disruptive", data: { reflex: "click->products#clear_search" } } + %a{ href: "#", class: "button disruptive medium", data: { reflex: "click->products#clear_search" } } = t(".pagination.clear_search") %div.with-dropdown = t(".pagination.per_page.show") diff --git a/app/webpacker/css/admin/products_v3.scss b/app/webpacker/css/admin/products_v3.scss index e7bd48858d..1a08f8a966 100644 --- a/app/webpacker/css/admin/products_v3.scss +++ b/app/webpacker/css/admin/products_v3.scss @@ -91,11 +91,16 @@ align-items: center; } - #sort .with-dropdown { - display: flex; - justify-content: space-between; - align-items: center; - gap: 10px; + #sort { + line-height: $btn-medium-height; + height: $btn-medium-height; + + .with-dropdown { + display: flex; + justify-content: space-between; + align-items: center; + gap: 10px; + } } #filters { diff --git a/app/webpacker/css/admin_v3/components/buttons.scss b/app/webpacker/css/admin_v3/components/buttons.scss index 5ad38c345a..eca539fa6c 100644 --- a/app/webpacker/css/admin_v3/components/buttons.scss +++ b/app/webpacker/css/admin_v3/components/buttons.scss @@ -78,6 +78,11 @@ button:not(.plain):not(.trix-button), } } + &.medium { + line-height: $btn-medium-height; + height: $btn-medium-height; + } + .badge { position: absolute; top: 0; diff --git a/app/webpacker/css/admin_v3/globals/variables.scss b/app/webpacker/css/admin_v3/globals/variables.scss index 1efa62199a..3c9ef5faa7 100644 --- a/app/webpacker/css/admin_v3/globals/variables.scss +++ b/app/webpacker/css/admin_v3/globals/variables.scss @@ -149,3 +149,4 @@ $font-weight-bold: 600 !default; $font-weight-normal: 400 !default; $btn-height: 40px !default; +$btn-medium-height: 32px !default; From fac02c794c0f5c3f81c180a7b3dc88c7f5d7c48d Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 5 Jul 2023 14:28:34 +0200 Subject: [PATCH 32/49] Search through more attributes than only name Copy/paste from `app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee` --- app/reflexes/products_reflex.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 537a7b9d3b..52c8f96e5b 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -107,7 +107,11 @@ class ProductsReflex < ApplicationReflex def ransack_query query = { s: "name desc" } query = query.merge({ supplier_id_in: @producer_id }) if @producer_id.present? - query = query.merge({ name_cont: @search_term }) if @search_term.present? + if @search_term.present? + # rubocop:disable Layout/LineLength + query = query.merge({ name_or_meta_keywords_or_variants_display_as_or_variants_display_name_or_supplier_name_cont: @search_term }) + # rubocop:enable Layout/LineLength + end query = query.merge({ primary_taxon_id_in: @category_id }) if @category_id.present? query end From dec779a3574ef2737eaeac105e601fb7bc971aae Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 12 Jul 2023 08:50:24 +0200 Subject: [PATCH 33/49] `available_on` column has been deleted https://github.com/openfoodfoundation/openfoodnetwork/pull/11136 --- app/views/admin/products_v3/_table.html.haml | 6 ------ config/locales/en.yml | 1 - 2 files changed, 7 deletions(-) diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index b582da532f..facf18c5d8 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -8,7 +8,6 @@ %col{ width:"10%" } %col{ width:"5%" } %col{ width:"5%", style: "max-width:5em" } - %col{ width:"8%", style: "max-width:8em" } %thead %tr %th.align-left= t('admin.product.name') @@ -20,7 +19,6 @@ %th.align-left= t('admin.category') %th.align-left= t('admin.tax_category') %th.align-left= t('admin.inherits_properties') - %th.align-right= t('admin.available_on') - products.each do |product| %tbody.relaxed %tr @@ -46,8 +44,6 @@ .line-clamp-1= product.tax_category&.name %td.align-left .line-clamp-1= 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 - .line-clamp-1= product.available_on&.strftime('%F') - product.variants.each do |variant| %tr.condensed %td.align-left @@ -68,6 +64,4 @@ .line-clamp-1= variant.tax_category&.name %td.align-left .line-clamp-1= variant.product.inherits_properties ? 'YES' : 'NO' # same as product - %td.align-right - .line-clamp-1= variant.available_on&.strftime('%F') diff --git a/config/locales/en.yml b/config/locales/en.yml index 682e090f86..b89752a37c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -4258,7 +4258,6 @@ See the %{link} to find out more about %{sitename}'s features and to start using category: Category tax_category: Tax Category inherits_properties?: Inherits Properties? - available_on: Available On av_on: "Av. On" import_date: "Import Date" products_variant: From 037589ecda70b49ffebb9136a7183f66004811ac Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 12 Jul 2023 09:24:08 +0200 Subject: [PATCH 34/49] Add columns translation --- app/views/admin/products_v3/_table.html.haml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/app/views/admin/products_v3/_table.html.haml b/app/views/admin/products_v3/_table.html.haml index facf18c5d8..f5f14fad7d 100644 --- a/app/views/admin/products_v3/_table.html.haml +++ b/app/views/admin/products_v3/_table.html.haml @@ -10,15 +10,15 @@ %col{ width:"5%", style: "max-width:5em" } %thead %tr - %th.align-left= t('admin.product.name') - %th.align-right= t('admin.sku') - %th.align-right= t('admin.unit') - %th.align-right= t('admin.price') - %th.align-right= t('admin.on_hand') - %th.align-left= t('admin.producer') - %th.align-left= t('admin.category') - %th.align-left= t('admin.tax_category') - %th.align-left= t('admin.inherits_properties') + %th.align-left= t('admin.products_page.columns.name') + %th.align-right= t('admin.products_page.columns.sku') + %th.align-right= t('admin.products_page.columns.unit') + %th.align-right= t('admin.products_page.columns.price') + %th.align-right= t('admin.products_page.columns.on_hand') + %th.align-left= t('admin.products_page.columns.producer') + %th.align-left= t('admin.products_page.columns.category') + %th.align-left= t('admin.products_page.columns.tax_category') + %th.align-left= t('admin.products_page.columns.inherits_properties') - products.each do |product| %tbody.relaxed %tr From 755a1842c24cb395764929bf70e7745d4c66d86d Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 12 Jul 2023 09:33:42 +0200 Subject: [PATCH 35/49] Simplify to save execution time --- spec/system/admin/products_v3/products_spec.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index f7b521ec27..5c23950f5f 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -37,15 +37,9 @@ describe 'As an admin, I can see the new product page' do visit "/admin/products_v3" end - it "has a pagination" do + it "has a pagination, has 15 products per page by default and can change the page" do expect(page).to have_selector ".pagination" - end - - it "has 15 products per page by default" do expect_products_count_to_be 15 - end - - it "can change the page" do within ".pagination" do click_link "2" end From b3bdba3a9bdfdeea88d77906174cfb45b201647a Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 12 Jul 2023 09:38:07 +0200 Subject: [PATCH 36/49] Better use `merge!` instead of `q = q.merge` --- app/reflexes/products_reflex.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 52c8f96e5b..631789d8f6 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -106,13 +106,13 @@ class ProductsReflex < ApplicationReflex def ransack_query query = { s: "name desc" } - query = query.merge({ supplier_id_in: @producer_id }) if @producer_id.present? + query.merge!(supplier_id_in: @producer_id) if @producer_id.present? if @search_term.present? # rubocop:disable Layout/LineLength - query = query.merge({ name_or_meta_keywords_or_variants_display_as_or_variants_display_name_or_supplier_name_cont: @search_term }) + query.merge!(name_or_meta_keywords_or_variants_display_as_or_variants_display_name_or_supplier_name_cont: @search_term) # rubocop:enable Layout/LineLength end - query = query.merge({ primary_taxon_id_in: @category_id }) if @category_id.present? + query.merge!(primary_taxon_id_in: @category_id) if @category_id.present? query end From 90c63981972460e475b2c773340199852a18f6d2 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 12 Jul 2023 09:40:25 +0200 Subject: [PATCH 37/49] Simplify and call methods directly via `before_reflex` --- app/reflexes/products_reflex.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 631789d8f6..1ee59c901c 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -3,7 +3,7 @@ class ProductsReflex < ApplicationReflex include Pagy::Backend - before_reflex :init_params + before_reflex :init_filters_params, :init_pagination_params def fetch fetch_and_render_products @@ -33,11 +33,6 @@ class ProductsReflex < ApplicationReflex private - def init_params - init_filters_params - init_pagination_params - end - def init_filters_params # params comes from the form # _params comes from the url From b42dfb574b50a6229a6dddc926347650e2d3fc18 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 12 Jul 2023 09:41:41 +0200 Subject: [PATCH 38/49] Delete already imported (and modified) file --- app/webpacker/css/admin_v3/all.scss | 1 - 1 file changed, 1 deletion(-) diff --git a/app/webpacker/css/admin_v3/all.scss b/app/webpacker/css/admin_v3/all.scss index 7b666a4f6d..2350e97121 100644 --- a/app/webpacker/css/admin_v3/all.scss +++ b/app/webpacker/css/admin_v3/all.scss @@ -51,7 +51,6 @@ @import "../admin/components/actions"; @import "../admin/components/alert-box"; @import "../admin/components/alert_row"; -@import "../admin/components/buttons"; @import "components/buttons"; // admin_v3 @import "../admin/components/date-picker"; @import "../admin/components/dialogs"; From 3245f7ff998e5adf4ea36a303b24234f81bdfb56 Mon Sep 17 00:00:00 2001 From: jibees Date: Wed, 12 Jul 2023 09:34:53 +0200 Subject: [PATCH 39/49] Update app/reflexes/products_reflex.rb Co-authored-by: David Cook --- app/reflexes/products_reflex.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 1ee59c901c..15e7311771 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -70,11 +70,9 @@ class ProductsReflex < ApplicationReflex end def producers - producers = if current_user.has_spree_role?("admin") - Enterprise.all - else - current_user.enterprises - end + producers = [{ label: "All", value: "all" }] + + OpenFoodNetwork::Permissions.new(@user) + .managed_product_enterprises.is_primary_producer.by_name producers.map { |p| [p.name, p.id] } end From 2697a637a92bb4223ea0f8bde94f543f82525e83 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 12 Jul 2023 09:43:54 +0200 Subject: [PATCH 40/49] Use the `OpenFoodNetwork::Permissions` helper To retrieve only primary producers --- app/reflexes/products_reflex.rb | 5 ++--- spec/system/admin/products_v3/products_spec.rb | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 15e7311771..323b3af729 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -70,9 +70,8 @@ class ProductsReflex < ApplicationReflex end def producers - producers = [{ label: "All", value: "all" }] + - OpenFoodNetwork::Permissions.new(@user) - .managed_product_enterprises.is_primary_producer.by_name + producers = OpenFoodNetwork::Permissions.new(current_user) + .managed_product_enterprises.is_primary_producer.by_name producers.map { |p| [p.name, p.id] } end diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index 5c23950f5f..87774f4186 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -14,7 +14,7 @@ describe 'As an admin, I can see the new product page' do # create a product with a supplier that can be searched let!(:product_by_supplier) { create(:simple_product, - supplier: create(:enterprise, name: "Producer 1")) + supplier: create(:enterprise, name: "Producer 1", is_primary_producer: true)) } # create a product with a category that can be searched let!(:product_by_category) { From 79421f2265da1db6f2bcd63e9e04e577ac746323 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 12 Jul 2023 10:13:26 +0200 Subject: [PATCH 41/49] Expect to found the product --- spec/system/admin/products_v3/products_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index 87774f4186..69e3037b59 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -109,6 +109,7 @@ describe 'As an admin, I can see the new product page' do expect(page).to have_select "category_id", selected: "Category 1" expect_page_to_be 1 expect_products_count_to_be 1 + expect(page).to have_selector "table.products tbody tr td", text: product_by_category.name end end @@ -118,6 +119,7 @@ describe 'As an admin, I can see the new product page' do expect(page).to have_field "search_term", with: "searchable product" expect_page_to_be 1 expect_products_count_to_be 1 + expect(page).to have_selector "table.products tbody tr td", text: product_by_name.name click_link "Clear search" expect(page).to have_field "search_term", with: "" From 3fab9714f530e21ab9029af8f4a663062eab50df Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 17 Jul 2023 14:46:40 +0200 Subject: [PATCH 42/49] Use a constant as the search_key --- app/models/spree/variant.rb | 6 ++++++ app/reflexes/products_reflex.rb | 4 +--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index df7a588f14..0c98ba005d 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -21,6 +21,12 @@ module Spree NAME_FIELDS = ["display_name", "display_as", "weight", "unit_value", "unit_description"].freeze + SEARCH_KEY = "#{%w(name + meta_keywords + variants_display_as + variants_display_name + supplier_name).join('_or_')}_cont".freeze + belongs_to :product, -> { with_deleted }, touch: true, class_name: 'Spree::Product' delegate_belongs_to :product, :name, :description, :tax_category_id, :shipping_category_id, diff --git a/app/reflexes/products_reflex.rb b/app/reflexes/products_reflex.rb index 323b3af729..803d0e7c54 100644 --- a/app/reflexes/products_reflex.rb +++ b/app/reflexes/products_reflex.rb @@ -100,9 +100,7 @@ class ProductsReflex < ApplicationReflex query = { s: "name desc" } query.merge!(supplier_id_in: @producer_id) if @producer_id.present? if @search_term.present? - # rubocop:disable Layout/LineLength - query.merge!(name_or_meta_keywords_or_variants_display_as_or_variants_display_name_or_supplier_name_cont: @search_term) - # rubocop:enable Layout/LineLength + query.merge!(Spree::Variant::SEARCH_KEY => @search_term) end query.merge!(primary_taxon_id_in: @category_id) if @category_id.present? query From 6f6de009190978426f0e685e5b4d56459ba64958 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 19 Jul 2023 10:45:13 +0200 Subject: [PATCH 43/49] Better naming of argument to not override existing variable... --- spec/system/admin/products_v3/products_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index 69e3037b59..e457fe4a23 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -137,9 +137,8 @@ describe 'As an admin, I can see the new product page' do end end - def expect_page_to_be(page) - pending "this test is not working" - expect(page).to have_selector ".pagination .page.current", text: page.to_s + def expect_page_to_be(page_number) + expect(page).to have_selector ".pagination span.page.current", text: page_number.to_s end def expect_per_page_to_be(per_page) From 55cad3659f4f519612e2654c818f1e8a9da94a11 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 19 Jul 2023 11:19:08 +0200 Subject: [PATCH 44/49] Simplify products creation --- spec/system/admin/products_v3/products_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index e457fe4a23..976f4bf24c 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -8,14 +8,14 @@ describe 'As an admin, I can see the new product page' do include FileHelper # create lot of products - let!(:products) { create_list(:simple_product, 70) } + 70.times do |i| + let!("product_#{i}".to_sym) { create(:simple_product, name: "product #{i}") } + end # create a product with a name that can be searched let!(:product_by_name) { create(:simple_product, name: "searchable product") } # create a product with a supplier that can be searched - let!(:product_by_supplier) { - create(:simple_product, - supplier: create(:enterprise, name: "Producer 1", is_primary_producer: true)) - } + let!(:producer) { create(:supplier_enterprise, name: "Producer 1") } + let!(:product_by_supplier) { create(:simple_product, supplier: producer) } # create a product with a category that can be searched let!(:product_by_category) { create(:simple_product, taxons: [create(:taxon, name: "Category 1")]) From 62eaf4a1a924f8ebbc2ab1c78159891eb77a9992 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 19 Jul 2023 11:20:59 +0200 Subject: [PATCH 45/49] This spec is not flaky anymore --- spec/system/admin/products_v3/products_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index 976f4bf24c..e9c103fe69 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -71,7 +71,6 @@ describe 'As an admin, I can see the new product page' do end it "reset the page when searching" do - pending "this test is not working" within ".pagination" do click_link "2" end From bada2ef4a71ad6eaed8cd6d6b6ef3ff18611f44e Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 19 Jul 2023 14:27:57 +0200 Subject: [PATCH 46/49] Remove useless spec (will be tested right after) --- spec/system/admin/products_v3/products_spec.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index e9c103fe69..6ad07156d9 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -98,10 +98,6 @@ describe 'As an admin, I can see the new product page' do end context "search by category" do - it "has a category select" do - expect(page).to have_selector "select#category_id" - end - it "can search for a product" do search_by_category "Category 1" From 5d4ef5b52cf829be4eb8ee974c006995d2bbee14 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 19 Jul 2023 14:28:26 +0200 Subject: [PATCH 47/49] To test the search by category, we need to use set `primary_taxon` attr --- spec/system/admin/products_v3/products_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/system/admin/products_v3/products_spec.rb b/spec/system/admin/products_v3/products_spec.rb index 6ad07156d9..c83e073fd9 100644 --- a/spec/system/admin/products_v3/products_spec.rb +++ b/spec/system/admin/products_v3/products_spec.rb @@ -18,7 +18,7 @@ describe 'As an admin, I can see the new product page' do let!(:product_by_supplier) { create(:simple_product, supplier: producer) } # create a product with a category that can be searched let!(:product_by_category) { - create(:simple_product, taxons: [create(:taxon, name: "Category 1")]) + create(:simple_product, primary_taxon: create(:taxon, name: "Category 1")) } before do From 2a81a9acfb2cadec1eb6aeeb8c8199e479d3dde4 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Mon, 24 Jul 2023 16:40:25 +0200 Subject: [PATCH 48/49] Add ability for non super-admin user to see products_v3 page --- app/models/spree/ability.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index 9ffa81bae9..1da1374b9d 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -191,6 +191,8 @@ module Spree OpenFoodNetwork::Permissions.new(user).managed_product_enterprises.include? product.supplier end + can [:admin, :index], :products_v3 + can [:create], Spree::Variant can [:admin, :index, :read, :edit, :update, :search, :delete, :destroy], Spree::Variant do |variant| From 352cf2ff7157d89c89ee2421a7f5288955c7d998 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 25 Jul 2023 14:29:00 +0200 Subject: [PATCH 49/49] Import typography.scss for admin_v3 And do not apply `a` style to `.button` element. ie. ``` a:not(.button) ``` --- app/webpacker/css/admin_v3/all.scss | 2 +- .../css/admin_v3/components/buttons.scss | 1 + .../css/admin_v3/shared/typography.scss | 226 ++++++++++++++++++ 3 files changed, 228 insertions(+), 1 deletion(-) create mode 100644 app/webpacker/css/admin_v3/shared/typography.scss diff --git a/app/webpacker/css/admin_v3/all.scss b/app/webpacker/css/admin_v3/all.scss index 2350e97121..4cbc555727 100644 --- a/app/webpacker/css/admin_v3/all.scss +++ b/app/webpacker/css/admin_v3/all.scss @@ -29,7 +29,7 @@ @import "../shared/variables/layout"; @import "../shared/variables/variables"; @import "../shared/utilities"; -@import "../admin/shared/typography"; +@import "shared/typography"; // admin_v3 @import "shared/tables"; // admin_v3 @import "shared/icons"; // admin_v3 @import "../admin/shared/forms"; diff --git a/app/webpacker/css/admin_v3/components/buttons.scss b/app/webpacker/css/admin_v3/components/buttons.scss index eca539fa6c..57683f72a6 100644 --- a/app/webpacker/css/admin_v3/components/buttons.scss +++ b/app/webpacker/css/admin_v3/components/buttons.scss @@ -14,6 +14,7 @@ button:not(.plain):not(.trix-button), text-transform: uppercase; line-height: 40px; height: 40px; + font-weight: bold; &:before { font-weight: normal !important; diff --git a/app/webpacker/css/admin_v3/shared/typography.scss b/app/webpacker/css/admin_v3/shared/typography.scss new file mode 100644 index 0000000000..467ee248be --- /dev/null +++ b/app/webpacker/css/admin_v3/shared/typography.scss @@ -0,0 +1,226 @@ +// Base +//-------------------------------------------------------------- +body, +div, +dl, +dt, +dd, +ul, +ol, +li, +h1, +h2, +h3, +h4, +h5, +h6, +pre, +form, +p, +blockquote, +th, +td { + margin: 0; + padding: 0; + font-size: $body-font-size; +} + +body { + font-family: $base-font-family; + font-size: $body-font-size; + font-weight: 400; + color: $color-body-text; + text-rendering: optimizeLegibility; +} + +hr { + border-top: 1px solid $color-border; + border-bottom: 1px solid white; + border-left: none; +} + +strong, +b { + font-weight: 600; +} + +// links +//-------------------------------------------------------------- +a:not(.button) { + color: $color-link; + text-decoration: none; + line-height: inherit; + + &, + &:hover, + &:active, + &:visited, + &:focus { + outline: none; + } + + &:visited { + color: $color-link-visited; + } + &:focus { + color: $color-link-focus; + } + &:active { + color: $color-link-active; + } + &:hover { + color: $color-link-hover; + } +} + +// Headings +//-------------------------------------------------------------- + +h1, +h2, +h3, +h4, +h5, +h6 { + font-weight: 600; + color: $color-headers; + line-height: 1.1; +} + +h1 { + font-size: $h1-size; + line-height: $h1-size + 6; +} +h2 { + font-size: $h2-size; + line-height: $h1-size + 4; +} +h3 { + font-size: $h3-size; + line-height: $h1-size + 2; +} +h4 { + font-size: $h4-size; + line-height: $h1-size; +} +h5 { + font-size: $h5-size; + line-height: $h1-size; +} +h6 { + font-size: $h6-size; + line-height: $h1-size; +} + +// Lists +//-------------------------------------------------------------- +ul { + &.inline-menu { + li { + display: inline-block; + } + } + &.fields { + list-style: none; + padding: 0; + margin: 0; + } +} + +dl { + width: 100%; + overflow: hidden; + margin: 5px 0; + color: lighten($color-body-text, 15); + + dt, + dd { + float: left; + line-height: 16px; + padding: 5px; + text-align: justify; + } + + dt { + width: 40%; + font-weight: 600; + padding-left: 0; + text-transform: uppercase; + font-size: 85%; + } + + dd { + width: 60%; + padding-right: 0; + } + + dd:after { + content: ""; + clear: both; + } +} + +// Helpers +.align-center { + text-align: center; +} +.align-right { + text-align: right; +} +.align-left { + text-align: left; +} +.align-justify { + text-align: justify; +} + +.uppercase { + text-transform: uppercase; +} + +.green { + color: $color-2; +} +.blue { + color: $color-3; +} +.red { + color: $color-5; +} +.yellow { + color: $color-6; +} + +.no-objects-found { + text-align: center; + font-size: 120%; + text-transform: uppercase; + padding: 40px 0px; + color: lighten($color-body-text, 15); +} + +.text-normal { + font-size: 1rem; + font-weight: 300; +} + +.text-big { + font-size: 1.2rem; + font-weight: 300; +} + +.text-red { + color: $warning-red; +} + +input.text-big { + font-size: 1.1rem; +} + +.pad-top { + padding-top: 1em; +} + +.white-space-nowrap { + white-space: nowrap; +}