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

Cart transform can send the title override for cart #3304

Closed
wants to merge 1 commit into from
Closed

Cart transform can send the title override for cart #3304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 sections/cart-notification-product.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
{%- if settings.show_vendor -%}
<p class="caption-with-letter-spacing light">{{ item.product.vendor }}</p>
{%- endif -%}
<h3 class="cart-notification-product__name h4">{{ item.product.title | escape }}</h3>
<h3 class="cart-notification-product__name h4">{{ item.title | escape }}</h3>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this create a bit of an ugly fallback that isn't wanted.
Here is a comparison of the before vs after:

screenshot

We're already mentioning the variant/options picked in a nicer visual way and this is repeating that content.

We should involve a theme UX designer to get their opinion on what would be best here.

It doesn't seem like we can tell when a transform has been applied or not right ? There isn't logic we could apply to it to only use item.title when a transform is detected 🤔

cc: @melissaperreault

Copy link
Author

@lanadz-shopify lanadz-shopify Feb 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We're already mentioning the variant/options picked in a nicer visual way and this is repeating that content.

Thanks for feedback! Yeah, agree that this might be not the nicest representation.

There isn't logic we could apply to it to only use item.title when a transform is detected 🤔

We might expand and add such logic to liquid. Or introduce new entire property for cart item, for instance item.presentment_title
Another option would be can redefine item.title to be the same as item.product.title instead of concatenation of product title + variant title
From representing point of view, what is the purpose of cart.item.title? would it make sense to be the same as product title for dawn or other themes?

Copy link
Contributor

@melissaperreault melissaperreault Feb 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

+1 to @ludoboludo's concerns: repeating the variant name in the product title is redundant for all buyers, including buyers using voice over. I am sure there are more clever outcome we can explore 🙏

For example, when selling plans are available, it would be listed as a line item under the variant choices details; both in cart and checkout.

Curious to learn where this lands!

Copy link
Author

Choose a reason for hiding this comment

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

Thank you @melissaperreault and @ludoboludo!
Example with selling plans makes much sense, we probably should look into some conditional rendering here as well.

contextually for existing representation it makes sense to keep item.product.title and with listed variant details under. For all merchants that not using cart transform it is not needed regression. Overriding product.title doesn't seem right either, as we are not overriding product's title, but cart item's title.

I will need to speak with product about this one, but suspect the best solution would be probably to introduce a new field here to have something like item.title_override? and item.title_override for this specific use case.

cc @dpwolf @ignacio-chiazzo

Copy link
Author

Choose a reason for hiding this comment

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

@dpwolf I think we might need some UX advice here,
would it make sense to have some kind of different behaviour in case if we have title override?

have something like this in default case (current behaviour)

- product title
   - variant title 1
   - variant title 2

and totally different rendering for case when we have title overrides, like skipping the variants values and have only

title override for line instead of product title - variant title 1/variant title 2

Should it even be on theme by default? or those changes have to be available for customization for each merchant for them to decide?

<dl>
{%- unless item.product.has_only_default_variant -%}
{%- for option in item.options_with_values -%}
Expand Down
8 changes: 4 additions & 4 deletions sections/main-cart-items.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
<p class="caption-with-letter-spacing">{{ item.product.vendor }}</p>
{%- endif -%}

<a href="{{ item.url }}" class="cart-item__name h4 break">{{ item.product.title | escape }}</a>
<a href="{{ item.url }}" class="cart-item__name h4 break">{{ item.title | escape }}</a>

{%- if item.original_price != item.final_price -%}
<div class="cart-item__discounted-prices">
Expand Down Expand Up @@ -243,7 +243,7 @@
<quantity-input class="quantity cart-quantity">
<button class="quantity__button no-js-hidden" name="minus" type="button">
<span class="visually-hidden">
{{- 'products.product.quantity.decrease' | t: product: item.product.title | escape -}}
{{- 'products.product.quantity.decrease' | t: product: item.title | escape -}}
</span>
{% render 'icon-minus' %}
</button>
Expand All @@ -261,13 +261,13 @@
{% endif %}
step="{{ item.variant.quantity_rule.increment }}"
{% # theme-check-enable %}
aria-label="{{ 'products.product.quantity.input_label' | t: product: item.product.title | escape }}"
aria-label="{{ 'products.product.quantity.input_label' | t: product: item.title | escape }}"
id="Quantity-{{ item.index | plus: 1 }}"
data-index="{{ item.index | plus: 1 }}"
>
<button class="quantity__button no-js-hidden" name="plus" type="button">
<span class="visually-hidden">
{{- 'products.product.quantity.increase' | t: product: item.product.title | escape -}}
{{- 'products.product.quantity.increase' | t: product: item.title | escape -}}
</span>
{% render 'icon-plus' %}
</button>
Expand Down
8 changes: 4 additions & 4 deletions snippets/cart-drawer.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@
{%- endif -%}

<a href="{{ item.url }}" class="cart-item__name h4 break">
{{- item.product.title | escape -}}
{{- item.title | escape -}}
</a>

{%- if item.original_price != item.final_price -%}
Expand Down Expand Up @@ -282,7 +282,7 @@
<span class="visually-hidden">
{{-
'products.product.quantity.decrease'
| t: product: item.product.title
| t: product: item.title
| escape
-}}
</span>
Expand All @@ -302,15 +302,15 @@
{% endif %}
step="{{ item.variant.quantity_rule.increment }}"
{% # theme-check-enable %}
aria-label="{{ 'products.product.quantity.input_label' | t: product: item.product.title | escape }}"
aria-label="{{ 'products.product.quantity.input_label' | t: product: item.title | escape }}"
id="Drawer-quantity-{{ item.index | plus: 1 }}"
data-index="{{ item.index | plus: 1 }}"
>
<button class="quantity__button no-js-hidden" name="plus" type="button">
<span class="visually-hidden">
{{-
'products.product.quantity.increase'
| t: product: item.product.title
| t: product: item.title
| escape
-}}
</span>
Expand Down
Loading