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] website_sale_browse_mode #1014

Open
wants to merge 3 commits into
base: 16.0
Choose a base branch
from

Conversation

victor-champonnois
Copy link
Member

@victor-champonnois victor-champonnois commented Jan 29, 2025

@victor-champonnois victor-champonnois force-pushed the 16.0-add-website_sale_browse_mode branch from a788054 to fbe060a Compare January 29, 2025 15:10
@victor-champonnois victor-champonnois marked this pull request as draft January 29, 2025 15:16
@victor-champonnois victor-champonnois force-pushed the 16.0-add-website_sale_browse_mode branch from fbe060a to 4c3ddf1 Compare January 29, 2025 16:44
@victor-champonnois victor-champonnois marked this pull request as ready for review January 29, 2025 16:49
Copy link
Contributor

@remytms remytms left a comment

Choose a reason for hiding this comment

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

Great, nice feature and very simple. :)

Some points:

  • Not sure if the name is clear enough. Perhaps something like website_sale_browse_only or website_sale_disable_shopping or website_sale_no_shopping, etc. But maybe it’s just me that did catch directly the meaning.
  • I think that the case when the cart is not empty when the website shift to "browse only" should be take into account. In such case the user will be able to finish the payment of his/her cart. Maybe button on the cart view should be removed also.

@victor-champonnois
Copy link
Member Author

@remytms
Thanks for the review

  • I hid the cart icon. It does not prevent someone who just add "/cart" to the url to access their card but it's a quick fix.
  • I renamed the module website_sale_browse_only

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.

2 participants