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

Quick Order List + Volume Pricing #2791

Merged
merged 31 commits into from
Jul 13, 2023
Merged

Quick Order List + Volume Pricing #2791

merged 31 commits into from
Jul 13, 2023

Conversation

sofiamatulis
Copy link
Contributor

@sofiamatulis sofiamatulis commented Jul 7, 2023

This PR introduces Volume Pricing and Quick Order List

Here are the PRs that were merged here (there are also some other PRs that were merged in the below PRs as well):

Editor: https://os2-demo.myshopify.com/admin/themes/140237864982/editor

@@ -330,6 +337,27 @@
},
"page": {
"title": "Page title"
},
"quick_order_list": {
"product_total": "Product subtotal",
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 Product subtotal for key sections.quick_order_list.product_total 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.

},
"quick_order_list": {
"product_total": "Product subtotal",
"view_cart": "View cart",
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 View cart for key sections.quick_order_list.view_cart 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.

"name": "Quick order list",
"settings": {
"show_image": {
"label": "Show images"
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 Show images for key sections.quick-order-list.settings.show_image.label 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.

"label": "Show images"
},
"show_sku": {
"label": "Show SKUs"
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 Show SKUs for key sections.quick-order-list.settings.show_sku.label 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.

@@ -330,6 +337,27 @@
},
"page": {
"title": "Page title"
},
"quick_order_list": {
"product_total": "Product subtotal",
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 Product subtotal for key sections.quick_order_list.product_total 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.

},
"quick_order_list": {
"product_total": "Product subtotal",
"view_cart": "View cart",
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 View cart for key sections.quick_order_list.view_cart 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.

"name": "Quick order list",
"settings": {
"show_image": {
"label": "Show images"
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 Show images for key sections.quick-order-list.settings.show_image.label 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.

@sofiamatulis sofiamatulis force-pushed the variantlist-volpricing branch from 294f9c3 to 7f498ad Compare July 12, 2023 20:18
@sofiamatulis sofiamatulis requested a review from KaichenWang July 12, 2023 20:19
@edmund-teh edmund-teh self-requested a review July 12, 2023 20:24
@sofiamatulis sofiamatulis requested a review from nicklepine July 12, 2023 20:29
Copy link

@nicklepine nicklepine left a comment

Choose a reason for hiding this comment

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

tophatted golden path via the link shared on the pr description. Looks great! Functional. Any tweaks can be made subsequently as a hotfix on dawn/main.

@sofiamatulis sofiamatulis requested a review from KaichenWang July 12, 2023 21:48
@KaichenWang
Copy link
Contributor

We'll need to do more testing with global styles in general after merging, but for now wanted to flag that some of the volume pricing popover buttons will inherit global button border styles. We can wait to fix in main if that makes more sense.

Screenshot 2023-07-12 at 5 57 34 PM Screenshot 2023-07-12 at 5 58 05 PM

@edmund-teh
Copy link

edmund-teh commented Jul 13, 2023

@sofiamatulis

Everything's looking really good!

+1 to @KaichenWang's comment on testing Global styles— let's wait to address those in main...

@sofiamatulis
Copy link
Contributor Author

Fixed ^ https://screenshot.click/13-35-krmv1-84bik.png
https://screenshot.click/13-36-lauaq-0kh38.png

Copy link

@edmund-teh edmund-teh left a comment

Choose a reason for hiding this comment

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

LGTM (main)!

@scottgmeadows
Copy link

Bug: Variant list add to cart breaks if you click it 2 short bursts.

Repro steps:
1 - Click + icon on a quantity picker
2 - Pause
3 - Click it a bunch of times really fast.
4 - Result: Error and unexpected quantity.

Suggested solution: Block quantity changes when loading the same way we do in cart.

breaking-add-to-cart-variant-list.mp4

@sofiamatulis sofiamatulis force-pushed the variantlist-volpricing branch from 16d516e to 6b09409 Compare July 13, 2023 18:43
@PaulNewton
Copy link

Is this feature demoed anywhere on the actual dawn demo store
https://theme-dawn-demo.myshopify.com ?
Either just the variant-list or the volume-pricing, or both.

@eugenekasimov
Copy link
Contributor

Hey @PaulNewton Quick order list and volume pricing are in the current version of Dawn now. To see them you should be logged in as a B2B customer. In order to see volume-pricing you need to browse the website via preview and not the editor.

@PaulNewton
Copy link

@eugenekasimov private instances are moot and come with their own baggage for public demos.

The theme store specifically notes the quick-order-list as a headlining feature in the changelog for v11.
How would one register as B2B on a shopify DEMO store that doesn't have a wholesale application, if it even has the feature enabled at all.
https://themes.shopify.com/themes/dawn/styles/default
Nor does it mention B2B as a requirement in the features bullet point in the theme store changelog

People on the outside considering shopify or discussing theme capabilities cannot validate the growing list of claimed features in shopify's own feature-lists without jumping through unnecessary hoops.
Obviously not every subfeature can be in a demo but there waaaay too many tentpole features not on display in a singular place for evaluation.

phapsidesGT pushed a commit to Gravytrain-UK/gt-shopify-dawn-theme that referenced this pull request Sep 3, 2024
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.

7 participants