💄 [#3016] Desktop search filters - new design #1632
Conversation
stefrado
commented
Feb 21, 2025
•
stefrado
commented
- ✨ Hide filters if there are no choices available.
- 💄 Implement new search filter UI.
- 🎨 Open always open the first filter on page load.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1632 +/- ##
===========================================
- Coverage 94.20% 94.15% -0.06%
===========================================
Files 1083 1087 +4
Lines 39955 40100 +145
===========================================
+ Hits 37641 37756 +115
- Misses 2314 2344 +30 ☔ View full report in Codecov by Sentry. |
| @@ -4,6 +4,10 @@ | |||
| flex-direction: column; | |||
| gap: var(--spacing-large); | |||
|
|
|||
| @media (min-width: 768px) { | |||
| margin-top: 3.375rem; // 54px | |||
There was a problem hiding this comment.
I agree REMs are better but the project is pixelbased. Also: we need to be able to adjust spaces in future with design-tokens. So municipalities can adjust spacing themselves (if they use different font etc.)
Using calc(2 * var(--spacing-giant) - var(--spacing-small)) could be another solution
| .button { | ||
| height: fit-content; | ||
| padding: 0; | ||
| margin-bottom: 0.75rem; // 12px |
There was a problem hiding this comment.
Again not sure if REMs are the way to go when municipalities will need to change margin-values with design-tokens.
This one has a tricky top-margin. In the design the distance between "Wis alle?" and the text with number of Resultaten (search-results__title-small) is exactly 60 pixels, but will need to be adjust due to inner padding etc.
| content: ''; | ||
| display: block; | ||
| background-image: url("data:image/svg+xml,%3Csvg width='20' height='21' viewBox='0 0 20 21' fill='none' xmlns='http://www.w3.org/2000/svg'%3E%3Cg clip-path='url(%23clip0_2541_1098)'%3E%3Cpath d='M15.1263 5.84163L10.8215 10.1464L10.468 10.5L10.8215 10.8535L15.1263 15.1583L14.6584 15.6262L10.3536 11.3214L10.0001 10.9679L9.64653 11.3214L5.34175 15.6262L4.87386 15.1583L9.17863 10.8535L9.53219 10.5L9.17863 10.1464L4.87385 5.84163L5.34175 5.37373L9.64653 9.67851L10.0001 10.0321L10.3536 9.67851L14.6584 5.37373L15.1263 5.84163Z' fill='white' stroke='white'/%3E%3C/g%3E%3Cdefs%3E%3CclipPath id='clip0_2541_1098'%3E%3Crect width='20' height='20' fill='white' transform='translate(0 0.5)'/%3E%3C/clipPath%3E%3C/defs%3E%3C/svg%3E%0A"); | ||
| width: 1.25rem; |
There was a problem hiding this comment.
Width and height could perhaps better be var(--font-size-body-large), since we usually use the Material-Icons font for these, but I know there is no Close icon in UTF 😞
There was a problem hiding this comment.
Will update the value to the var()!
| @@ -319,6 +333,8 @@ def _test_search(checkbox_name, expected_text): | |||
| expected_text | |||
| ) | |||
|
|
|||
| print("Tag 1", self.product1.name) | |||
There was a problem hiding this comment.
The 'name' should be coming from self.tag1 = TagFactory.create(name="Tag 1") I think?
I'd really ask @pi-sigma if he can help out with this failing test.
Since all the proper elements seem to be there, so I am unsure how to solve this 😞
| {% if search_filter_organizations and search_form.organizations.field.choices|length != 0 %} | ||
| {% include "components/Filter/Filter.html" with field=search_form.organizations form_id="search-form" initial_open=open_index|is_same:2 only %} | ||
| {% endif %} | ||
| {% search_filters search_form form_id="search-form" search_filter_categories search_filter_tags search_filter_organizations %} |
There was a problem hiding this comment.
Keyword arguments (form_id="search-form") must come after positional arguments (search_filter_categories etc.). This causes a TemplateSyntaxError which in turn causes expect(page.locator(".search-results__item")).to_have_count(2) to fail.
| @@ -312,7 +312,6 @@ def _click_checkbox_for_name(page=page, name=''): | |||
|
|
|||
| # Open het <details>-element | |||
| # details_element.click() | |||
There was a problem hiding this comment.
You need to query the detail_element with get_by_text and pass in the name to get the correct locator:
details_element = page.locator(".filter").filter(has=page.get_by_text(name))
details_element.click()
This will make the relevant details_element (a different one for each call to _click_checkbox_for_name visible
stefrado
force-pushed
the
feature/3016-desktop-search-filter
branch
2 times, most recently
from
74a11bb to
710cc93
Compare
stefrado
force-pushed
the
feature/3016-desktop-search-filter
branch
from
710cc93 to
e72dca0
Compare
