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

Fixed issues when header is hidden #3545

Merged
merged 3 commits into from
Jul 15, 2024
Merged

Fixed issues when header is hidden #3545

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e303b8c
Fixed issues when header is hidden
tyleralsbury Jul 3, 2024
e1de801
Fixed predictive search on search page
tyleralsbury Jul 3, 2024
0942e6a
Made it so that the drawer still shows up
tyleralsbury Jul 12, 2024
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
2 changes: 1 addition & 1 deletion assets/predictive-search.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ class PredictiveSearch extends SearchForm {

getResultsMaxHeight() {
this.resultsMaxHeight =
window.innerHeight - document.querySelector('.section-header').getBoundingClientRect().bottom;
window.innerHeight - document.querySelector('.section-header')?.getBoundingClientRect().bottom;
return this.resultsMaxHeight;
}

Expand Down
12 changes: 12 additions & 0 deletions layout/theme.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@
<script src="{{ 'constants.js' | asset_url }}" defer="defer"></script>
<script src="{{ 'pubsub.js' | asset_url }}" defer="defer"></script>
<script src="{{ 'global.js' | asset_url }}" defer="defer"></script>
<script src="{{ 'details-disclosure.js' | asset_url }}" defer="defer"></script>
<script src="{{ 'details-modal.js' | asset_url }}" defer="defer"></script>
<script src="{{ 'search-form.js' | asset_url }}" defer="defer"></script>

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder here if we should also add:

{%- if settings.cart_type == "drawer" -%}
  <script src="{{ 'cart-drawer.js' | asset_url }}" defer="defer"></script>
{%- endif -%}

Which is in the header right now. So that if you click on Add to cart then the drawer is shown anyway?
If we're already including the styles assets, we might as well do the script too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Goooood point. I just updated it - took a bit of a change to the code in drawer (since it has an awareness of the header), but wasn't too tricky.

{%- if settings.animations_reveal_on_scroll -%}
<script src="{{ 'animations.js' | asset_url }}" defer="defer"></script>
{%- endif -%}
Expand Down Expand Up @@ -253,14 +257,22 @@

{{ 'base.css' | asset_url | stylesheet_tag }}

{%- if settings.cart_type == 'drawer' -%}
{{ 'component-cart-drawer.css' | asset_url | stylesheet_tag }}
{{ 'component-cart.css' | asset_url | stylesheet_tag }}
{{ 'component-totals.css' | asset_url | stylesheet_tag }}
{{ 'component-price.css' | asset_url | stylesheet_tag }}
{{ 'component-discounts.css' | asset_url | stylesheet_tag }}
{%- endif -%}

{%- unless settings.type_body_font.system? -%}
{% comment %}theme-check-disable AssetPreload{% endcomment %}
<link rel="preload" as="font" href="{{ settings.type_body_font | font_url }}" type="font/woff2" crossorigin>

Check warning on line 270 in layout/theme.liquid

View workflow job for this annotation

GitHub Actions / Theme Check Report

layout/theme.liquid#L270 

[AssetPreload] For better performance, prefer using the preload_tag filter
{% comment %}theme-check-enable AssetPreload{% endcomment %}
{%- endunless -%}
{%- unless settings.type_header_font.system? -%}
{% comment %}theme-check-disable AssetPreload{% endcomment %}
<link rel="preload" as="font" href="{{ settings.type_header_font | font_url }}" type="font/woff2" crossorigin>

Check warning on line 275 in layout/theme.liquid

View workflow job for this annotation

GitHub Actions / Theme Check Report

layout/theme.liquid#L275 

[AssetPreload] For better performance, prefer using the preload_tag filter
{% comment %}theme-check-enable AssetPreload{% endcomment %}
{%- endunless -%}

Expand Down
13 changes: 3 additions & 10 deletions sections/header.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,15 @@
<link rel="stylesheet" href="{{ 'component-menu-drawer.css' | asset_url }}" media="print" onload="this.media='all'">
<link rel="stylesheet" href="{{ 'component-cart-notification.css' | asset_url }}" media="print" onload="this.media='all'">
<link rel="stylesheet" href="{{ 'component-cart-items.css' | asset_url }}" media="print" onload="this.media='all'">

{%- if settings.predictive_search_enabled -%}
<link rel="stylesheet" href="{{ 'component-price.css' | asset_url }}" media="print" onload="this.media='all'">
{%- endif -%}

{%- if section.settings.menu_type_desktop == 'mega' -%}
<link rel="stylesheet" href="{{ 'component-mega-menu.css' | asset_url }}" media="print" onload="this.media='all'">
{%- endif -%}

{%- if settings.cart_type == "drawer" -%}
{{ 'component-cart-drawer.css' | asset_url | stylesheet_tag }}
{{ 'component-cart.css' | asset_url | stylesheet_tag }}
{{ 'component-totals.css' | asset_url | stylesheet_tag }}
{{ 'component-price.css' | asset_url | stylesheet_tag }}
{{ 'component-discounts.css' | asset_url | stylesheet_tag }}
{%- endif -%}

<style>
header-drawer {
Expand Down Expand Up @@ -102,10 +97,8 @@
}
{%- endstyle -%}

<script src="{{ 'details-disclosure.js' | asset_url }}" defer="defer"></script>
<script src="{{ 'details-modal.js' | asset_url }}" defer="defer"></script>
<script src="{{ 'cart-notification.js' | asset_url }}" defer="defer"></script>
<script src="{{ 'search-form.js' | asset_url }}" defer="defer"></script>

{%- if settings.cart_type == "drawer" -%}
<script src="{{ 'cart-drawer.js' | asset_url }}" defer="defer"></script>
{%- endif -%}
Expand Down
2 changes: 1 addition & 1 deletion sections/predictive-search.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,13 @@
tabindex="-1"
>
{%- if product.featured_media != blank -%}
<img
class="predictive-search__image"
src="{{ product.featured_media | image_url: width: 150 }}"
alt="{{ product.featured_media.alt }}"
alt="{{ product.featured_media.alt | escape }}"
width="50"
height="{{ 50 | divided_by: product.featured_media.preview_image.aspect_ratio }}"
>

Check warning on line 151 in sections/predictive-search.liquid

View workflow job for this annotation

GitHub Actions / Theme Check Report

sections/predictive-search.liquid#L145-L151 

[ImgLazyLoading] Use loading="eager" for images visible in the viewport on load and loading="lazy" for others
{%- endif -%}
<div class="predictive-search__item-content{% unless settings.predictive_search_show_vendor or settings.predictive_search_show_price %} predictive-search__item-content--centered{% endunless %}">
{%- if settings.predictive_search_show_vendor -%}
Expand Down
Loading