-
Notifications
You must be signed in to change notification settings - Fork 361
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.
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
feat(checkout): CHECKOUT-000 Add spans for product-title, product-item-quantity, product-option-label, and product-option-value #1701
base: master
Are you sure you want to change the base?
Conversation
…m-quantity, product-option-label, and product-option-value
@@ -37,7 +39,8 @@ const OrderSummaryItem: FunctionComponent<OrderSummaryItemProps> = ({ | |||
className="product-title optimizedCheckout-contentPrimary" | |||
data-test="cart-item-product-title" | |||
> | |||
{`${quantity} x ${name}`} | |||
<span className="product-item-quantity">{quantity}</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to change this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically no. But the community elaborated, and mentioned that it's the same improvement and would be nice to be able to target qty and name separately as well. Enabling dev's to style those individually with CSS without the need to introduce JS to do so
@@ -46,7 +49,8 @@ const OrderSummaryItem: FunctionComponent<OrderSummaryItemProps> = ({ | |||
> | |||
{productOptions.map((option, index) => ( | |||
<li className="product-option" data-test={option.testId} key={index}> | |||
{option.content} | |||
<span className="product-option-label">{option.optionLabel}</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a check here for content, just in case for customisations? Something like
{Boolean(content) ?
{option.content} :
<>
<span className="product-option-label">{option.optionLabel}</span>
<span className="product-option-value">{option.optionValue}</span>
</>
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
option.content
is removed all together from the product options and replaced by optionLabel
and optionValue
…m-quantity, product-option-label, and product-option-value
What?
As requested by community members here, #1700
we've added wrapping spans around product quantity and product title, as well as adding wrapping spans around product options label and value in the cart-summary.
Why?
This improves customization options for developers as they can target these values now purely with CSS
Testing / Proof
@bigcommerce/team-checkout