Pre-select current tax category on product form #3936
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kennyadsl
reviewed
Feb 15, 2021
| @@ -158,7 +158,7 @@ | |||
| <%= f.field_container :tax_category do %> | |||
| <%= f.label :tax_category_id, Spree::TaxCategory.model_name.human %> | |||
| <%= f.field_hint :tax_category, default_tax_category: @default_tax_category&.name %> | |||
| <%= f.collection_select(:tax_category_id, @tax_categories, :id, :name, { include_blank: t('spree.match_choices.none'), selected: @default_tax_category&.id }, { class: 'custom-select' }) %> | |||
| <%= f.collection_select(:tax_category_id, @tax_categories, :id, :name, { include_blank: t('spree.match_choices.none'), selected: f.object.tax_category_id || @default_tax_category&.id }, { class: 'custom-select' }) %> | |||
There was a problem hiding this comment.
What about setting the default on @product.tax_category at the controller level?
There was a problem hiding this comment.
sure, that's an option. Both solutions look rather equivalent to me. While the view gets simpler (we can entirely remove the selected specification here) the controller gains a before_action for both edit and new.
I've updated the code, let me know what you think!
There was a problem hiding this comment.
I like this way! Let see if others have opinions as well.
The `tax_category` select on the product form page should pre-select the default category only when the product does not have a tax category yet. If it does, then the current product tax category should be pre-selected, instead of the default one.
spaghetticode
force-pushed
the
spaghetticode/fix-product-tax-category-select
branch
from
28eb286
to
b30c04b
Compare
kennyadsl
approved these changes
Feb 15, 2021
jarednorman
approved these changes
Feb 15, 2021
kennyadsl
added
Needs Backport
changelog:solidus_backend
|
Hello all,
I cant under stand what you sent here ????
*Mohammad Ali*
*CEO & Founder | YalaShoghol*
Phone: *0792291616* <0792291616>
Website: *www.yalashoghol.com* <http://www.yalashoghol.com/>
Email: *[email protected]* <[email protected]>
Address: *King Hussein Business Park #7 First Floor Yalashoghol*
…On Tue, 16 Feb 2021 at 10:12, Alberto Vena ***@***.***> wrote:
Merged #3936 <#3936> into master.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#3936 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ARUIK3IL6JJLLOAXGR5PNUDS7ISF3ANCNFSM4XUPCOYA>
.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
tax_categoryselect on the product form page should pre-select the default category only when the product does not have a tax category yet. If it does, then the current product tax category should be pre-selected, instead of the default one.Checklist: