-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Admin UI for shipping methods - stock locations association #3624
Admin UI for shipping methods - stock locations association #3624
Conversation
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.
Thanks Dumi, this looks great!
@cedum thanks, this is awesome! I was wondering if we can easily clear and disable the Stock Locations field when Available to all is checked? It would be a nice UX addition. |
5e1f7bb
to
90c43e9
Compare
I've introduced a few interactions between the checkbox and the select:
What do you think? |
@cedum maybe it's not a good idea to auto-select the checkbox when clearing all stock locations? I'm thinking about a scenario where a store owner wants to temporarily disable a shipping method by removing it from all stock locations. They may not notice that the "Available to all stock locations" checkbox got auto-selected, and they would have the opposite result of what they were expecting. 😄 |
I think the scope of this feature is to support the main two cases UI-wise:
Actually, I'm thinking that the "Available to all" checkbox may be completely removed 😄 We can switch that attribute via a hidden input, it's mostly an implementation detail which probably should not be exposed outside. Disabling a shipping method by removing the stock locations whitelist and disabling the checkbox IMHO is an edge-case, and I think it may be a surprise for a user. If there's a need for such a feature, we may think about introducing a dedicated and clear action for that. What do you think? Not being a heavy Solidus admin user I may have misinterpreted something 😄 |
I think it makes sense — if the main use case is the whitelist, we can remove the user-facing "Available to all stock locations" input and just add a hint under the "Stock locations" input. |
90c43e9
to
5bd9fa1
Compare
When trying to transform the "Available to all" checkbox into a hidden input I took and ported @DanielePalombo suggestion from an old PR he was experimenting with. The Daniele's approach I think is better and way less confusing. Here's a demo, hopefully, the last one 😅 |
5bd9fa1
to
f94d107
Compare
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.
Awesome @cedum, just one small nitpick on the wording for the checkbox hint!
f94d107
to
bcc8733
Compare
Introduce the admin UI to be able to associate a shipping method to specific stock locations. This makes editable also `Spree::ShippingMethod#available_to_all` from admin, being directly related to stock location association behaviour.
bcc8733
to
570a16a
Compare
This is amazing, thanks @cedum! |
This introduces the admin UI allowing to associate a shipping method to specific stock locations.
This makes editable also
Spree::ShippingMethod#available_to_all
from admin, being an attribute directly related to stock locations association behavior.This feature is mostly used when a shipping method must be available to a specific subset of stock locations.
The shipping methods for a package are queried using the following methods:
Spree::ShippingMethod.with_all_shipping_category_ids
Spree::ShippingMethod.available_in_stock_location
The second query method filters the shipping methods having
available_to_all
enabled OR having the stock location associated.By disabling
available_to_all
it becomes possible to make specific shipping methods available for specific stock locations.Checklist: