-
-
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
Allow changing calculator types and attributes without reloading page #1618
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
adammathys
approved these changes
Nov 25, 2016
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.
👍 Great change. Looks good to me.
tvdeyen
approved these changes
Nov 25, 2016
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 🤖
This allows us to change the type and assign attributes to the calculator at the same time. Otherwise it would attempt to build a calculator (with base class Spree::Calculator) when an id wasn't specified.
Previously, after changing the type of shipping calculator, the user would need to save the page before seeing or being able to edit any of the values for that calculator. This PR changes the calculator_fields partial to render all values for all types of preferences, and then hide and disable (which prevents form submission) the fields for calculator types other than the one selected.
Previously, after changing the type of promotion calculator, the user would need to save the page before seeing or being able to edit any of the values for that calculator. This PR changes the calculator_fields partial to render all values for all types of preferences, and then hide and disable (which prevents form submission) the fields for calculator types other than the one selected.
This fixes a couple issues: * Events are not bound to multiple times when initPromotionActions is called (called each time a promotion action is added) * Multiple tiered calculators can exist on a single page * Fewer events are listened to on the body, which can be a performance issue
jhawthorn
force-pushed
the
smart_calculator
branch
from
December 12, 2016 22:39
5b4f9a9
to
b2bcc7b
Compare
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.
Previously, when changing a calculator's type (or creating it for the first time), it was necessary to save and reload the edit page before seeing the calculator's attributes. Now it can all be updated in one go.
After
Before
Changes
To accomplish this all preference inputs for all calculator types are added to the page and are hidden and disabled by JS which hides them from the user and prevents their form submission.
The JS for the two "tiered" calculators needed to be refactored to not have issues when being re-initialized multiple times. This was also an issue with the existing behaviour, just less likely to be triggered.
The
accepts_nested_attributes_for
fromCalculatedAdjustments
hadupdate_only: true
added. This allows bothcalculator_type
andcalculator_attributes
to be set at the same time. It also has the side effect of not needing theid
specified incalculator_attributes
.