Skip to content

Commit

Permalink
Fixed issue #13712: Exclusive option does not exclude others
Browse files Browse the repository at this point in the history
  • Loading branch information
@lacrioque
lacrioque committed May 29, 2018
1 parent a042113 commit 74eaf56
Showing 1 changed file with 16 additions and 4 deletions.
20 changes: 16 additions & 4 deletions assets/packages/limesurvey/survey.js
View file
Expand Up @@ -22,7 +22,9 @@ function triggerEmRelevance(){
function triggerEmRelevanceQuestion(){
/* Action on this question */
$("[id^='question']").on('relevance:on',function(event,data) {
if(event.target != this) return; /* @todo : attach only to this. Use http://stackoverflow.com/a/6411507/2239406 solution for now. Don't want to stop propagation. */
/* @todo : attach only to this. Use http://stackoverflow.com/a/6411507/2239406 solution for now.
Don't want to stop propagation. */
if(event.target != this) return;
$(this).removeClass("ls-irrelevant ls-hidden");
});
$("[id^='question']").on('relevance:off',function(event,data) {
Expand Down Expand Up @@ -59,7 +61,9 @@ function triggerEmRelevanceSubQuestion(){
data = $.extend({style:'hidden'}, data);
$(this).removeClass("ls-irrelevant ls-"+data.style);
if(data.style=='disabled'){
$(event.target).find('input').prop("disabled", false );
$(event.target).find('input').each(function(itrt, item ){
$(item).prop("disabled", false );
});
}
if(data.style=='hidden'){
updateLineClass($(this));
Expand All @@ -70,14 +74,22 @@ function triggerEmRelevanceSubQuestion(){
if(event.target != this) return; // not needed now, but after (2016-11-07)
data = $.extend({style:'hidden'}, data);
$(this).addClass("ls-irrelevant ls-"+data.style);

if(data.style=='disabled'){
$(event.target).find('input').prop("disabled", true );
$(event.target).find('input')
.each(function(itrt, item ){
if($(item).attr('type') == 'checkbox' && $(item).prop('checked')) {
$(item).prop('checked', false).trigger('change');
}
$(item).prop("disabled", true );
});
}

if(data.style=='hidden'){
updateLineClass($(this));
updateRepeatHeading($(this).closest(".ls-answers"));
}
$(this).find('input[disabled]').prop('checked', false);
console.ls.log($(this).find('input[disabled]'));
});
}
Expand Down

5 comments on commit 74eaf56

@Shnoulle
Copy link
Collaborator

@Shnoulle Shnoulle commented on 74eaf56 May 29, 2018 •
edited

Choose a reason for hiding this comment

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

since we attach the event to the line ($("[id^='question']").on('relevance:off',"[id^='javatbd']") why need an each ?

And in 2.73 and before, if user click on exclusive option : it doens't uncheck other checkbox : because when uncheck exclusive option : have their checkbox checked like before. I'm sure we have a bug about this asking to "don't uncheck other checkbox with exclusive option".

Em find value of a checkbox with relevanceStatus + current state, if relevanceStatus is 0 : return false.

@lacrioque
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need an each, because there is as well the comment input as also the checkbox input inside of the question container.
I wasn't aware, that the exclusive option shouldn't uncheck the other options. I will fix that.

@Shnoulle
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can just send the trigger('change'); event, this launch EM. But : with 50 sub question : we launch EM 50 times in one click :/

@lacrioque
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you trigger change on the element you have an infinite trigger loop. I tested that.

@Shnoulle
Copy link
Collaborator

@Shnoulle Shnoulle commented on 74eaf56 May 30, 2018 •
edited

Choose a reason for hiding this comment

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

Yes,

I know :

            .each(function(itrt, item ){
                if($(item).attr('type') == 'checkbox' ) {
                    $(item).trigger('change');
                }

trigger only on the input, not on the line of the subquestion :), no ? (didn't test)

About checkbox : array have exclusive-option too

Please sign in to comment.