-
-
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
Add an HTML select element to filter orders by the shipment state #4089
Add an HTML select element to filter orders by the shipment state #4089
Conversation
Spec failures seem legit:
Can you please address this? Thanks! |
Closed by mistake 🤦 Done, @kennyadsl. Thanks. |
This looks good. Do you mind squashing the commits together? |
85f06d3
to
238ccb5
Compare
I squashed both commits, @jarednorman. Thanks. |
Thanks! Looks good. |
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.
Left one comment, thanks!
@@ -48,6 +48,10 @@ | |||
<%= f.text_field :shipments_number_start %> | |||
</div> | |||
|
|||
<div class="field"> | |||
<%= label_tag :q_shipment_state, t('spree.shipment_state') %> | |||
<%= f.select :shipment_state_eq, %i[backorder canceled partial pending ready shipped].map { |s| [t("spree.shipment_states.#{s}"), s] }, { include_blank: true }, { class: "custom-select fullwidth" } %> |
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.
I prefer to have a longer line (especially because it's already very long) but without the single-letter variable. Can we please change s
into state
?
238ccb5
to
15af307
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.
Thanks!
Description
Add an HTML select element to filter orders by the shipment state.
Fixes #4023
Checklist: