Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix no-js layout for drawer and product count position #1572

Merged
merged 6 commits into from
Apr 4, 2022
Merged

Fix no-js layout for drawer and product count position #1572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ad6861a
fix no-js layout for drawer and product count position
martinamarien Apr 1, 2022
df52f3c
udpate vertical product count position on drawer on desktop
martinamarien Apr 1, 2022
072b344
fix broken html
martinamarien Apr 1, 2022
9a427e3
remove unnecessary facets class
martinamarien Apr 1, 2022
e4ade8f
update some logic to fix state after rebase
martinamarien Apr 1, 2022
7a222b0
fix spacing
martinamarien Apr 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 7 additions & 28 deletions assets/component-facets.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -992,11 +992,11 @@ input.mobile-facets__checkbox {
padding-right: 3rem;
}

.js .facets-vertical .facets-wrapper.no-filter {
.facets-vertical .facets-wrapper--no-filters {
display: none;
}

.no-js .facets-vertical .facets-wrapper.no-filter {
.no-js .facets-vertical .facets-wrapper--no-filters {
display: block;
}
Comment on lines +995 to 1001
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my testing, the added specificity was not required, so I used a modifier and removed the .js class from the first style declaration.

Copy link
Contributor

@KaichenWang KaichenWang Apr 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 also if the .no-filter class isn't meant to be added/removed to reflect a state change (which seems to be the case here), it's better to use a modifier like you did.


Expand Down Expand Up @@ -1040,8 +1040,9 @@ input.mobile-facets__checkbox {
margin-bottom: 1.5rem;
}

.facets-vertical .facet-filters.sorting.no-js {
.no-js .facets-vertical .facet-filters.sorting {
padding-left: 0;
flex-direction: column;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose to move no-js to the form for now. Even though I couldn't access it with a screenreader, it didn't have display: none.


.facets-vertical .facet-checkbox input[type='checkbox'] {
Expand Down Expand Up @@ -1069,27 +1070,17 @@ input.mobile-facets__checkbox {
.facets-container-drawer {
display: flex;;
flex-flow: row wrap;
align-items: center;
column-gap: 0;
}

.facets-container-drawer .mobile-facets__wrapper {
margin-right: 2rem;
flex-grow: 1;
}

.facets-container-drawer .product-count {
align-self: flex-start;
}

.facets-container-drawer .mobile-facets__wrapper {
width: calc(40% - 2rem);
}

.facets-container-drawer .facets {
width: 40%;
}

.facets-container-drawer .product-count {
width: 20%;
margin:0 0 0.5rem 3.5rem;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@melissaperreault This should fix the Drawer Product count alignment.

}

.facets-container-drawer .facets-pill {
Expand All @@ -1100,18 +1091,6 @@ input.mobile-facets__checkbox {
.facets-container-drawer .facets__form {
display: block;
}

.no-js .facets-container-drawer .mobile-facets__wrapper {
width: calc(20% - 2rem);
}

.no-js .facets-container-drawer .facets {
width: 60%;
}

.no-js .facets-container-drawer .product-count {
width: 20%;
}
}

@media screen and (min-width: 750px) and (max-width: 989px) {
Expand Down
2 changes: 1 addition & 1 deletion sections/main-collection-product-grid.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
{{ 'component-facets.css' | asset_url | stylesheet_tag }}
<script src="{{ 'facets.js' | asset_url }}" defer="defer"></script>
{%- if section.settings.enable_filtering or section.settings.enable_sorting -%}
<aside aria-labelledby="verticalTitle" class="facets-wrapper{% if section.settings.enable_filtering == false %} no-filter{% endif %}{% if section.settings.filter_type != 'vertical' %} page-width{% endif %}" id="main-collection-filters" data-id="{{ section.id }}">
<aside aria-labelledby="verticalTitle" class="facets-wrapper{% unless section.settings.enable_filtering %} facets-wrapper--no-filters{% endunless %}{% if section.settings.filter_type != 'vertical' %} page-width{% endif %}" id="main-collection-filters" data-id="{{ section.id }}">
{% render 'facets', results: collection, enable_filtering: section.settings.enable_filtering, enable_sorting: section.settings.enable_sorting, filter_type: section.settings.filter_type %}
</aside>
{%- endif -%}
Expand Down
2 changes: 1 addition & 1 deletion sections/main-search.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@
<div{% if section.settings.filter_type == 'vertical' %} class="facets-vertical page-width"{% endif %}>
{%- if search.filters != empty -%}
{%- if section.settings.enable_filtering or section.settings.enable_sorting -%}
<aside aria-labelledby="verticalTitle" class="facets-wrapper{% if section.settings.enable_filtering == false %} no-filter{% endif %}{% if section.settings.filter_type != 'vertical' %} page-width{% endif %}" id="main-search-filters" data-id="{{ section.id }}">
<aside aria-labelledby="verticalTitle" class="facets-wrapper{% unless section.settings.enable_filtering %} facets-wrapper--no-filters{% endunless %}{% if section.settings.filter_type != 'vertical' %} page-width{% endif %}" id="main-search-filters" data-id="{{ section.id }}">
{% render 'facets', results: search, enable_filtering: section.settings.enable_filtering, enable_sorting: section.settings.enable_sorting, filter_type: section.settings.filter_type %}
</aside>
{%- endif -%}
Expand Down
8 changes: 4 additions & 4 deletions snippets/facets.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,9 +353,9 @@
</facet-filters-form>
{% comment %} Sorting for vertical filter are grouped with filter when no JS{% endcomment %}
{%- if enable_sorting and filter_type == 'vertical' -%}
<facet-filters-form>
<form>
<div class="facet-filters sorting caption no-js small-hide">
<facet-filters-form class="small-hide">
<form class="no-js">
<div class="facet-filters sorting caption">
<div class="facet-filters__field">
<h2 class="facet-filters__label caption-large text-body">
<label for="SortBy">{{ 'products.facets.sort_by_label' | t }}</label>
Expand All @@ -376,7 +376,7 @@
</noscript>
</div>
</form>
<facet-filters-form>
</facet-filters-form>
{%- endif -%}
{%- endif -%}
{% comment %} Drawer and mobile filter {% endcomment %}
Expand Down