Use attribute translations for customer return forms #764
Conversation
|
👍 |
| @@ -33,7 +33,7 @@ | |||
| </fieldset> | |||
|
|
|||
| <%= f.field_container :stock_location do %> | |||
| <%= f.label :stock_location, Spree.t(:stock_location) %> | |||
| <%= f.label :stock_location_id %> | |||
There was a problem hiding this comment.
IMO we should use Spree::StockLocation.model_name.human here. This avoids duplicated translations.
There was a problem hiding this comment.
Explicitly state a different model's attribute to avoid adding a translation for the attribute to the model? This just seems kind of what the opposite of what I thought we were going for.
There was a problem hiding this comment.
Its the name of the other model. Its referencing what StockLocation it belongs_to, so referencing what a StockLocation is by the way of that table is perfectly reasonable
There was a problem hiding this comment.
Though an added benefit besides eliminating the code would be this stops the select2 label bug.
There was a problem hiding this comment.
Alright, sounds good to me I'll make the change. Just wanted a little clarification.
There was a problem hiding this comment.
To be more clear about this:
<%= f.label :stock_location, Spree::StockLocation.model_name.human %>If this also fixes the select2 label problem, great
There was a problem hiding this comment.
I'd just made it <%= f.label Spree::StockLocation.model_name.human %> and it's working fine. Everyone happy with that? Or go for the more verbose option?
Murph33
force-pushed
the
use_attribute_translations_customer_return_forms
branch
from
65c5580
to
fa5d342
Compare
| total: Total | ||
| created_at: "Date/Time" | ||
| reimbursement_status: Reimbursement status | ||
| stock_location_id: Stock Location |
There was a problem hiding this comment.
This is not needed anymore. See comments above
|
@Murph33 sorry about the confusion. |
Murph33
force-pushed
the
use_attribute_translations_customer_return_forms
branch
from
fa5d342
to
c6feb74
Compare
Murph33
force-pushed
the
use_attribute_translations_customer_return_forms
branch
4 times, most recently
from
cb0cfa6
to
a4025f3
Compare
Change-Id: I2b41ae32237622edef509809457efaea0f9b7529
Murph33
force-pushed
the
use_attribute_translations_customer_return_forms
branch
from
a4025f3
to
38de18e
Compare
Required to pass tests as there was a missing translation before.
Murph33
force-pushed
the
use_attribute_translations_customer_return_forms
branch
from
38de18e
to
8d5d825
Compare
|
I'm 👍 on this, and my understanding is the feedback here is resolved as well. |
|
👍 |
…mer_return_forms Use attribute translations for customer return forms
This is to better utilize I18n with more verbosity to make it clear and easier to translate. By using attributes from the model it will be much more clear what is happening instead of taking the translation from "some place" in the dictionary.
This is cherry picked from #549 and is part of an ongoing chain of PRs as discussed in #735 to improve the I18n implementation.
The only changes from the commit is not erasing things in the dictionary and adding
item_received?to the ReturnItem section.