Skip to content
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

Pass previous operator to afterUpdateRuleOperator event #547

Merged
merged 1 commit into from
Nov 18, 2017

Conversation

giero
Copy link

@giero giero commented Aug 10, 2017

Added previousOperator to 'afterUpdateRuleOperator' event - could be useful in some cases ;)

@mistic100
Copy link
Owner

Could also do it for afterUpdateRuleFilter, afterUpdateRuleValue and afterUpdateGroupCondition ? (these needs to update the method calls on core.js lines 285 and 300)

@giero
Copy link
Author

giero commented Aug 11, 2017

I'm on it, but ... updateRuleValue is called also in updateRuleOperator function (just after afterUpdateRuleOperator event) - what should be passed there?
New updateRuleValue will look like this

QueryBuilder.prototype.updateRuleValue = function(rule, oldValue) {
...
}

but updateRuleOperator doesn't change the rule value ... so ... undefined?

EDIT: Well - it changes value in one case:

if (!rule.operator || rule.operator.nb_inputs === 0) { ... }

so I will copy value of rule.__.value before it changes and pass this value to the updateRuleValue function.

@giero
Copy link
Author

giero commented Aug 16, 2017

@mistic100 Is it enough to merge?

@mistic100
Copy link
Owner

There is a pending review.

@mistic100
Copy link
Owner

@giero there is still a pending review

src/core.js Outdated
@@ -723,6 +726,7 @@ QueryBuilder.prototype.updateRuleFilter = function(rule, previousFilter) {
*/
QueryBuilder.prototype.updateRuleOperator = function(rule, previousOperator) {
var $valueContainer = rule.$el.find(QueryBuilder.selectors.value_container);
var ruleValue = rule.__.value;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can just use rule.value, __ is only useful to set the value without triggering an update event

@mistic100
Copy link
Owner

@giero Sorry the review was not published, I didn't see it because the behavior changed some months ago

@giero
Copy link
Author

giero commented Nov 7, 2017

@mistic100
I've changed what you suggested.
It's only few lines of code - why review last so long?

@mistic100 mistic100 merged commit fa0be59 into mistic100:dev Nov 18, 2017
@mistic100 mistic100 added the feature New feature label Nov 18, 2017
@mistic100 mistic100 added this to the 2.4.5 milestone Nov 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants