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.

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 support for new combination options #84

Merged
merged 5 commits into from
Aug 8, 2023
Merged

Conversation

davejcameron
Copy link
Contributor

@davejcameron davejcameron commented Aug 3, 2023

Closes #83

Adds support for the new combination options that are available here: https://help.shopify.com/en/manual/discounts/combining-discounts/discount-combinations#discount-combination-examples

  • Matches the new selection options based on the type
  • Updated the translation strings to match
  • Updated the storybook to show the different options to make testing easier
  • Added the warning banner

image

image

image

Copy link
Contributor

@lhoffbeck lhoffbeck left a comment

Choose a reason for hiding this comment

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

Left some comments on things that could be improved but no blockers. Thanks for the fix 🙏

"warning": {
"title": "Some combinations can result in large discounts",
"description": "Test a few combinations. If the total discount is too large, adjust which discounts can combine.",
"link": "Learn more"
Copy link
Contributor

Choose a reason for hiding this comment

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

Localization quality issue found

The following issues may affect the quality of localized translations if they are not addressed:

  • The value Learn more for key DiscountAppComponents.CombinationCard.warning.link is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.

Please look out for other instances of this issue in your PR and fix them as well if possible.

Questions about these messages? Hop in the #help-localization Slack channel.

Add in the warning banner
@mathiusj mathiusj merged commit 7a83719 into main Aug 8, 2023
4 checks passed
@mathiusj mathiusj deleted the update-combines-with branch August 8, 2023 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Update CombinationCard to support Product & Order discount combinations
3 participants