Back Office Product List - Hide search field and filters when there are no products in list #12983
Conversation
saunmanoj888
force-pushed
the
12957-hide-search-filter-for-no-products
branch
from
fa26cf4
to
d17c38c
Compare
There was a problem hiding this comment.
Part of me wonders if it should be more directly counting the number of producers that the user has access to. We could do that by re-factoring part of the query in ProductsV3Controller#fetch_products.
But this way seems reliable enough, and is actually more efficient 👍
| - if display_search_filter | ||
| .container | ||
| .sixteen.columns | ||
| = render partial: 'admin/shared/flashes', locals: { flashes: } if defined? flashes | ||
| = render partial: 'filters', locals: { search_term: search_term, | ||
| producer_id: producer_id, | ||
| producer_options: producer_options, | ||
| category_options: category_options, | ||
| category_id: category_id } |
There was a problem hiding this comment.
For future reference, when you have a conditional block of Haml code, it reads much nicer to move that code into a partial.
| - if display_search_filter | |
| .container | |
| .sixteen.columns | |
| = render partial: 'admin/shared/flashes', locals: { flashes: } if defined? flashes | |
| = render partial: 'filters', locals: { search_term: search_term, | |
| producer_id: producer_id, | |
| producer_options: producer_options, | |
| category_options: category_options, | |
| category_id: category_id } | |
| = render partial: "search_filter", locals: { flashes:, ... } if display_search_filter |
There are quite a few local though. Hm, looking at the code again, we probably don't want a conditional on the flashes. What if you delete the last product and that should have a success message? You would be hiding it in your code.
So maybe we want this?
| - if display_search_filter | |
| .container | |
| .sixteen.columns | |
| = render partial: 'admin/shared/flashes', locals: { flashes: } if defined? flashes | |
| = render partial: 'filters', locals: { search_term: search_term, | |
| producer_id: producer_id, | |
| producer_options: producer_options, | |
| category_options: category_options, | |
| category_id: category_id } | |
| .container | |
| .sixteen.columns | |
| = render partial: 'admin/shared/flashes', locals: { flashes: } if defined? flashes | |
| = render partial: 'filters', locals: { | |
| search_term:, | |
| producer_id:, | |
| producer_options:, | |
| category_options:, | |
| category_id: | |
| } if display_search_filter |
What do you think?
There was a problem hiding this comment.
hii @mkllnk
Agree with you, I have now added display_search_filter condition only for filters partial
For above suggested review, shorthand syntax works fine but curly brace placement gives obscure errors. For now I have kept the curly braces placement for key locals as it is.
Could you please review the latest changes
saunmanoj888
force-pushed
the
12957-hide-search-filter-for-no-products
branch
from
d17c38c
to
87d7f73
Compare
mkllnk
added
the
technical changes only
What? Why?
What should we test?
When a new person signs up and goes to the admin for the first time, they likely won't have any product, verify that in this scenario, search filter should not be present
When products are present and user searches with a product name which is not present in the list, search filter should still be visible
Release notes
Changelog Category (reviewers may add a label for the release notes):