-
-
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
[RFC] Introduce preference fields partials #2040
Conversation
1c59edf
to
0cf4d1d
Compare
0cf4d1d
to
83640b8
Compare
<%= form.check_box attribute, html_options %> | ||
<%= form.label attribute, label %> | ||
<% else %> | ||
<%= hidden_field_tag name, 0 %> |
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 wonder if we can simplify these cases using the new form_with
🤔
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.
Good question. Would be nice though. Will give it a try
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.
Turns out this would be tricky, because we would need to rewrite all forms rendering these fields. In terms of the calculator fields this means to rewrite the calculator forms. I think we should tackle this in a separate PR. WDYT?
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.
Yeah, let's tackle this later.
CHANGELOG.md
Outdated
@@ -1,5 +1,8 @@ | |||
## Solidus 2.3.0 (master, unreleased) | |||
|
|||
- Deprecates several preference fields helpers in favor of preference field partials. [\#2040](https://github.com/solidusio/solidus/pull/2040) ([tvdeyen](https://github.com/tvdeyen)) |
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.
Can we bump this forward to 2.4?
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.
Done
83640b8
to
3241507
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.
Yeah to explicit rendering!
3241507
to
a12383a
Compare
Preference fields are used in several places all over the Solidus admin. Some of them are: - Shipping and tax rate calculator fields - Payment method preferences - Promotion action calculator fields Instead of using a string concatinator helper - that just concatinate the fields and labels with `<br>` - we introduce partials that can easily be overriden by stores and extensions.
Instead of using a custom helper to render input fields for preferences of calculators we render the newly introduced preference fields partials.
This deprecates the follwoing preference fields helpers: - `preference_field_tag` - `preference_field_for` - `preference_field_options` - `preference_fields` Please render the fields for calculators and preferences directly.
Due to the newly introduced preference fields partials all preference fields are now wrapped inside its own field wrapper. Therefore we need to adjust the padding of the calculator preferences of promotion actions.
a12383a
to
8f690bb
Compare
Preference fields are used in several places all over the Solidus admin. Some of them are:
Instead of using a string concatinator helper - that just concatinate the fields and labels with
<br>
This deprecates the follwoing preference fields helpers:
preference_field_tag
preference_field_for
preference_field_options
preference_fields
Before
After