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

fix(Storefront): STRF-5072 Cornerstone not showing Sale badge #1694

Merged
merged 2 commits into from
Jun 30, 2020
Merged

fix(Storefront): STRF-5072 Cornerstone not showing Sale badge #1694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15f88b3
fix(Storefront): STRF-5072 Cornerstone not showing Sale badge in list…
yurytut1993 Jun 16, 2020
b7d81ad
master into STRF-5072
yurytut1993 Jun 16, 2020
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
8 changes: 3 additions & 5 deletions assets/scss/layouts/products/_productList.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,18 +64,15 @@

.listItem-figure {
margin: 0 0 spacing("single");

position: relative;

@include breakpoint("small") {
margin-bottom: 0;
padding-left: spacing("half");
padding-right: spacing("half");
width: grid-calc(3, $total-columns);
}

@include breakpoint("large") {
position: relative;
}

.listItem-button {
margin: spacing("single") 0;

Expand All @@ -87,6 +84,7 @@
transform-style: preserve-3d;
}
}

}

.listItem-figureBody {
Expand Down
34 changes: 34 additions & 0 deletions assets/scss/layouts/products/_productSaleBadges.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,23 @@
width: rem-calc(50px);
}

.listItem-figure {

.starwrap {
transform: scale(0.7);

@include breakpoint("small") {
top: 0;
transform: scale(0.6);
}

@include breakpoint("large") {
top: 10px;
transform: scale(0.7);
}
}
}

.sale-text-burst {
color: stencilColor("color_text_product_sale_badges");
font-weight: 600;
Expand Down Expand Up @@ -94,6 +111,23 @@
z-index: zIndex("lower");
}

.listItem-figure {

.sale-flag-sash {
left: -25px;

@include breakpoint("small") {
top: 15px;
}

@include breakpoint("large") {
left: -15px;
top: 25px;
}

}
}

.product:hover .sale-flag-sash {
background: stencilColor("color_hover_product_sale_badges");
transition: 800ms ease;
Expand Down
16 changes: 16 additions & 0 deletions templates/components/products/list-item.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,22 @@
<article class="listItem">
{{/if}}
<figure class="listItem-figure">
{{#or price.non_sale_price_with_tax price.non_sale_price_without_tax}}
{{#if theme_settings.product_sale_badges '===' 'sash'}}
<div class="sale-flag-sash">
<span class="sale-text">On Sale!</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we need these span tags ? Also I this is going to be surfaced in the theme the I think we need to move the text into schema.json and config.json so we merchants can modify them.

thoughts @bookernath @bc-as ??

Copy link
Contributor Author

@yurytut1993 yurytut1993 Jun 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

span tags used for text styles and aligning. We already have this HTML construction for sale badges in products grid. Of course we can add possibility to our merchants to edit sale badge text. But I suppose that we should to add an input for it in control panel firstly. To allow merchants set up it. In this pull request I just wanted to fix the bug with lack of sale badge in products list

Copy link
Contributor

Choose a reason for hiding this comment

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

@junedkazi we historically haven't exposed the text on those badges to be configurable, but it is a good idea. I'll open another ticket to address that feature.

Copy link
Contributor

Choose a reason for hiding this comment

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

https://jira.bigcommerce.com/browse/BCTHEME-61 for sales badge text

</div>
{{else if theme_settings.product_sale_badges '===' 'topleft'}}
<div class="sale-flag-side">
<span class="sale-text">On Sale!</span>
</div>
{{else if theme_settings.product_sale_badges '===' 'burst'}}
<div class="starwrap">
<div class="sale-text-burst">On Sale!</div>
<div class="sale-flag-star"></div>
</div>
{{/if}}
{{/or}}
{{> components/common/responsive-img
image=image
class="listItem-image"
Expand Down