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

Product card spacing and price polish #407

Merged
merged 3 commits into from
Aug 18, 2021
Merged

Product card spacing and price polish #407

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
dd228ba
Adjust product card spacing and price styles
kmeleta Aug 16, 2021
5f701b4
Fix unit price/sale badge spacing
kmeleta Aug 16, 2021
42da681
Revert flex gap approach
kmeleta Aug 17, 2021
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
10 changes: 8 additions & 2 deletions assets/component-card.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@
}

.card + .card-information {
margin-top: 1.2rem;
margin-top: 1.3rem;
}

@media screen and (min-width: 750px) {
.card + .card-information {
margin-top: 1.7rem;
}
}

.card.card--soft {
Expand Down Expand Up @@ -175,7 +181,7 @@
}

.card-information__wrapper > *:not(.visually-hidden:first-child) + * {
margin-top: 1.1rem;
margin-top: 0.7rem;
}

.card-information__wrapper .caption {
Expand Down
2 changes: 2 additions & 0 deletions assets/component-price.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
margin: 0 1rem 0 0;
}

.price dd:last-of-type,
.price .price__last:last-of-type {
margin: 0;
}
Expand Down Expand Up @@ -83,6 +84,7 @@
.price--on-sale .price-item--regular {
text-decoration: line-through;
color: rgba(var(--color-foreground), 0.75);
font-size: 1.3rem;
}

.unit-price {
Expand Down
22 changes: 13 additions & 9 deletions assets/customer.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,15 +473,6 @@
}
}

.order tbody td:nth-of-type(3) dd:nth-of-type(2) {
font-size: 1.1rem;
letter-spacing: 0.07rem;
line-height: 1.2;
margin-top: 0.2rem;
text-transform: uppercase;
color: var(--color-foreground-70);
}

.order tfoot tr:last-of-type td,
.order tfoot tr:last-of-type th {
font-size: 2.2rem;
Expand Down Expand Up @@ -617,6 +608,19 @@
color: rgba(var(--color-foreground), 0.7);
}

.order .unit-price {
font-size: 1.1rem;
letter-spacing: 0.07rem;
line-height: 1.2;
margin-top: 0.2rem;
text-transform: uppercase;
color: var(--color-foreground-70);
Copy link
Contributor

Choose a reason for hiding this comment

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

--color-foreground-XX CSS variables no longer exist in the theme. More context here.

Suggested change
color: var(--color-foreground-70);
color: rgba(var(--color-foreground), 0.7);

}

.order .regular-price {
font-size: 1.3rem;
}

/* Addresses */
.addresses li > button {
margin-left: 0.5rem;
Expand Down
5 changes: 5 additions & 0 deletions assets/section-main-product.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ a.product__text {

.product .price {
align-items: flex-start;
gap: 1rem;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This gap is to fix this issue which only happens at specific viewport widths for on sale items with unit pricing. I chose 1rem so it can take the place of the existing horizontal 1rem margin between the sale price and the sale badge (when displayed next to each on larger breakpoints).

Copy link
Contributor

Choose a reason for hiding this comment

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

I like this clean approach! But some versions of the latest iOS Safari 14 release (< 14.4) don't support this.

Screen Shot 2021-08-17 at 2 12 08 PM

Screen_Shot_2021-08-17_at_2_07_32_PM

}

.product .price--sold-out .price__badge-sold-out {
Expand All @@ -276,6 +277,10 @@ a.product__text {
}

@media screen and (min-width: 750px) {
.product__info-container .price--on-sale .price-item--regular {
font-size: 1.6rem;
}

.product__info-container > *:first-child {
margin-top: 0;
}
Expand Down
6 changes: 3 additions & 3 deletions templates/customers/order.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
<dt>
<span class="visually-hidden">{{ 'products.product.price.regular_price' | t }}</span>
</dt>
<dd>
<dd class="regular-price">
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember wanting to stir away from using classes in the customer templates. Those classes should be generic enough so it's ok to have them I take it ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think so. We do use a few other simple classes in this template as well as the account table for convenience and clarity. At a certain point a selector becomes too flaky and indiscernible to justify. This PR actually resolves a styling bug that presumably happened due to unintended selector scope.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sweet! And makes sense some selectors you removed were not pretty 😅

<s>{{ line_item.original_price | money }}</s>
</dd>
<dt>
Expand All @@ -150,7 +150,7 @@
<dt>
<span class="visually-hidden">{{ 'products.product.price.unit_price' | t }}</span>
</dt>
<dd>
<dd class="unit-price">
<span>
{%- capture unit_price_separator -%}
<span aria-hidden="true">/</span><span class="visually-hidden">{{ 'accessibility.unit_price_separator' | t }}&nbsp;</span>
Expand Down Expand Up @@ -187,7 +187,7 @@
<dt>
<span class="visually-hidden">{{ 'products.product.price.regular_price' | t }}</span>
</dt>
<dd>
<dd class="regular-price">
<s>{{ line_item.original_line_price | money }}</s>
</dd>
<dt>
Expand Down