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 a tooltip for default currency in store settings #5009

Merged

Conversation

kennyadsl
Copy link
Member

@kennyadsl kennyadsl commented Apr 12, 2023

Summary

This PR comes from some support requests we had in Slack, highlighting some confusion when a currency switch is required. I hope this will mitigate that confusion. By the way, this is just a little help, a proper page on the guide would be a good next step.

Screenshot 2023-04-12 at 11 27 55@2x

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📖 I have updated the README to account for my changes.
  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

@kennyadsl kennyadsl self-assigned this Apr 12, 2023
@kennyadsl kennyadsl requested a review from a team as a code owner April 12, 2023 06:01
@github-actions github-actions bot added changelog:solidus_backend Changes to the solidus_backend gem changelog:solidus_core Changes to the solidus_core gem labels Apr 12, 2023
@kennyadsl kennyadsl force-pushed the kennyadsl/tooltips-changing-currency branch from 7e0f945 to d158fef Compare April 12, 2023 06:31
Copy link
Contributor

@waiting-for-dev waiting-for-dev left a comment

Choose a reason for hiding this comment

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

Thanks, @kennyadsl! That confusion also hit me in the past.

@@ -1644,6 +1644,7 @@ en:
available_locales: This determines which locales are available for your customers to choose from in the storefront.
cart_tax_country_iso: 'This determines which country is used for taxes on carts (orders which don''t yet have an address).<br> Default: None.'
code: An identifier for your store. Developers may need this value if you operate multiple storefronts.
default_currency: This determines which currency will be used to retrieve product prices for this store. If you want to change it globally, set `Spree::Config.currency` instead. <br><br>Please, be aware that only products that have prices in the selected currency will be listed.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still a bit confused. What do we mean by "retrieve"? Do we mean from the frontend (although we should be agnostic)? Is it also true from the API? Do we mean the default product searcher? On top of that, I think (not 100% sure), that Spree::Config.currency is the only one used when creating products on the backend.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's used by the current_pricing_options helper, which is used by all frontends, API and base searcher, so I think it's safe to assume that it's a valid general rule for any "storefront" instance (in fact, it is available in the store preference).

It seems to be a different thing for the backend, and probably Spree::Config.currency is used to set the default currency because the same product can be available in different stores.

If you have any suggestions on how to make it clearer, please help me! 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

It's used by the current_pricing_options helper, which is used by all frontends, API and base searcher, so I think it's safe to assume that it's a valid general rule for any "storefront" instance (in fact, it is available in the store preference).

Cool. Then I think it's good enough in that regard.

It seems to be a different thing for the backend, and probably Spree::Config.currency is used to set the default currency because the same product can be available in different stores.

Do you think it is worth it to be explicit there? Like this setting won't change the default currency used when you create a product. For that, only Spree::Config.currency is taken into account.

Copy link
Member Author

Choose a reason for hiding this comment

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

Rephrased, what about now?

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool!

This is just a little help for people that try to switch currency.
A proper page on the guide would be a good next step.
@kennyadsl kennyadsl force-pushed the kennyadsl/tooltips-changing-currency branch from d158fef to 29a520a Compare April 12, 2023 09:02
Copy link
Contributor

@waiting-for-dev waiting-for-dev left a comment

Choose a reason for hiding this comment

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

❤️

@kennyadsl kennyadsl merged commit 523796e into solidusio:master Apr 14, 2023
@kennyadsl kennyadsl deleted the kennyadsl/tooltips-changing-currency branch April 14, 2023 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_backend Changes to the solidus_backend gem changelog:solidus_core Changes to the solidus_core gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants