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

Add unique ids to card-product.liquid headings and product state badges #1731

Merged
merged 7 commits into from
Jun 21, 2022
Merged

Add unique ids to card-product.liquid headings and product state badges #1731

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3bccc67
Update card-product.liquid
pizzaz93 May 23, 2022
a8d6363
Update card-product.liquid
pizzaz93 May 24, 2022
2bf55c7
Update card-product.liquid
pizzaz93 Jun 11, 2022
c3c1693
Update card-product.liquid
pizzaz93 Jun 13, 2022
b0794e1
Update card-product.liquid
pizzaz93 Jun 16, 2022
3b10a57
Update card-product.liquid
pizzaz93 Jun 16, 2022
dc4c1dc
Update snippets/card-product.liquid
pizzaz93 Jun 16, 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
14 changes: 7 additions & 7 deletions snippets/card-product.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
{{ card_product.media[1] | image_url }} {{ card_product.media[1].width }}w"
src="{{ card_product.media[1] | image_url: width: 533 }}"
sizes="(min-width: {{ settings.page_width }}px) {{ settings.page_width | minus: 130 | divided_by: 4 }}px, (min-width: 990px) calc((100vw - 130px) / 4), (min-width: 750px) calc((100vw - 120px) / 3), calc((100vw - 35px) / 2)"
alt="{{ card_product.media[1].alt | escape }}"
alt=""
class="motion-reduce"
loading="lazy"
width="{{ card_product.media[1].width }}"
Expand All @@ -86,24 +86,24 @@
<div class="card__content">
<div class="card__information">
<h3 class="card__heading"{% if card_product.featured_media == nil and settings.card_style == 'standard' %} id="title-{{ section_id }}-{{ card_product.id }}"{% endif %}>
<a href="{{ card_product.url }}" class="full-unstyled-link">
<a href="{{ card_product.url }}" id="cardLink-{{ section_id }}-{{ card_product.id }}" class="full-unstyled-link" aria-labelledby="cardLink-{{ section_id }}-{{ card_product.id }} badge-{{ section_id }}-{{ card_product.id }}">
Copy link
Contributor

Choose a reason for hiding this comment

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

We seem to have a bit of a mix with our approach on how to write IDs throughout the theme. But maybe you can default to start the id with a capital letter.
So CardLink-...
and Badge-...

Copy link
Contributor

Choose a reason for hiding this comment

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

oh and we can't have the ID duplicated like it is right now.
So I'd say the first one here could be StandardCardNoMediaLink-{{ section_id }}-{{ card_product.id }}.
It's quite verbose but it will at least be self explanatory to know when it's used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the ID, the badge ID on this line which should it be?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same for line 106

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ludoboludo

{{ card_product.title | escape }}
</a>
</h3>
</div>
<div class="card__badge {{ settings.badge_position }}">
{%- if card_product.available == false -%}
<span class="badge badge--bottom-left color-{{ settings.sold_out_badge_color_scheme }}">{{ 'products.product.sold_out' | t }}</span>
<span id="badge-{{ section_id }}-{{ card_product.id }}" class="badge badge--bottom-left color-{{ settings.sold_out_badge_color_scheme }}">{{ 'products.product.sold_out' | t }}</span>
{%- elsif card_product.compare_at_price > card_product.price and card_product.available -%}
<span class="badge badge--bottom-left color-{{ settings.sale_badge_color_scheme }}">{{ 'products.product.on_sale' | t }}</span>
<span id="badge-{{ section_id }}-{{ card_product.id }}" class="badge badge--bottom-left color-{{ settings.sale_badge_color_scheme }}">{{ 'products.product.on_sale' | t }}</span>
{%- endif -%}
</div>
</div>
</div>
<div class="card__content">
<div class="card__information">
<h3 class="card__heading{% if card_product.featured_media or settings.card_style == 'standard' %} h5{% endif %}"{% if card_product.featured_media or settings.card_style == 'card' %} id="title-{{ section_id }}-{{ card_product.id }}"{% endif %}>
<a href="{{ card_product.url }}" class="full-unstyled-link">
<a href="{{ card_product.url }}" id="cardLink-{{ section_id }}-{{ card_product.id }}" class="full-unstyled-link" aria-labelledby="cardLink-{{ section_id }}-{{ card_product.id }} badge-{{ section_id }}-{{ card_product.id }}">
{{ card_product.title | escape }}
</a>
</h3>
Expand Down Expand Up @@ -207,9 +207,9 @@
{%- endif -%}
<div class="card__badge {{ settings.badge_position }}">
{%- if card_product.available == false -%}
<span class="badge badge--bottom-left color-{{ settings.sold_out_badge_color_scheme }}">{{ 'products.product.sold_out' | t }}</span>
<span id="badge-{{ section_id }}-{{ card_product.id }}" class="badge badge--bottom-left color-{{ settings.sold_out_badge_color_scheme }}">{{ 'products.product.sold_out' | t }}</span>
{%- elsif card_product.compare_at_price > card_product.price and card_product.available -%}
<span class="badge badge--bottom-left color-{{ settings.sale_badge_color_scheme }}">{{ 'products.product.on_sale' | t }}</span>
<span id="badge-{{ section_id }}-{{ card_product.id }}" class="badge badge--bottom-left color-{{ settings.sale_badge_color_scheme }}">{{ 'products.product.on_sale' | t }}</span>
{%- endif -%}
</div>
</div>
Expand Down