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

Fixed issue #18543: Question Type "Array (texts)" crashes when in combination with "array filter", "Array filter style": disabled and "show totals" #2793

Conversation

gabrieljenik
Copy link
Collaborator

There were two problems:

  1. When sumTable set the total of the rows and columns, the jquery selector for updating the input field was based only on the "disabled" condition. It did not check that it was the input corresponding to the total, for example by class or id.

Under normal conditions, the only one disabled is the total, but when you use "Array Filter Style" = "disabled" there are more fields disabled, which created confusions.

It also created loops.
After the total field is updated, some events are triggered and the checkconditions is executed again.
When that was triggered for the "disabled" filtered fields, it created a loop.

  1. In the "relevance:on", it removed the disabled from all the entries in the row, including the total one, which should always be disabled.

The ticket error was caused by point 1, but without fixing point 2 it would still look bad, so I fixed both.

…bination with "array filter", "Array filter style": disabled and "show totals"
Copy link
Collaborator

@Shnoulle Shnoulle left a comment

Choose a reason for hiding this comment

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

Can you check with .answer-item input selector ?

assets/packages/limesurvey/survey.js Outdated Show resolved Hide resolved
…bination with "array filter", "Array filter style": disabled and "show totals"
Copy link
Collaborator

@Shnoulle Shnoulle left a comment

Choose a reason for hiding this comment

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

👍

Not tested : think must test array_filter with multiple choice, ranking etc …

@Shnoulle
Copy link
Collaborator

Seems test broken with multiple and radio

input stay disabled
limesurvey_survey_139438.lss.txt

@Shnoulle
Copy link
Collaborator

Shnoulle commented Dec 19, 2022

Seems test broken with multiple and radio

because event.target == <tag class="answer-item">

            if($(event.target).hasClass("answer-item")) {
                $(event.target).find('input').each(function(itrt, item ){
                    $(item).prop("disabled", false );
                });
            } else {
                $(event.target).find('.answer-item input').each(function(itrt, item ){
                    $(item).prop("disabled", false );
                });
            }

…bination with "array filter", "Array filter style": disabled and "show totals"
@gabrieljenik
Copy link
Collaborator Author

What do you think now?

@Shnoulle Shnoulle added Code review done Version checked for code issue without testing and removed Needs update by author Needs code review labels Dec 20, 2022
Copy link
Collaborator

@Shnoulle Shnoulle left a comment

Choose a reason for hiding this comment

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

Code review done,
To be tested again

@Shnoulle
Copy link
Collaborator

Shnoulle commented Dec 20, 2022

Firefox, asset resetted (i validate it's this file) : recursion again :(
limesurvey_survey_Filter4.lss.txt
Capture d’écran du 2022-12-20 18-35-27

@Shnoulle Shnoulle added Test failed Someone tested the PR and it did not work as expected. Please check out the comments. and removed Needs testing labels Dec 20, 2022
…bination with "array filter", "Array filter style": disabled and "show totals"

- Update minified script
@sonarcloud
Copy link

sonarcloud bot commented Dec 21, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Shnoulle
Copy link
Collaborator

Argl debug=0 issue ?

@Shnoulle Shnoulle added Tested OK This PR has been tested by QA and works as expected and removed Needs update by author Test failed Someone tested the PR and it did not work as expected. Please check out the comments. labels Dec 21, 2022
@Shnoulle
Copy link
Collaborator

OK !
Tested OK

@gabrieljenik
Copy link
Collaborator Author

OK ! Tested OK

OK, the issue was the minified file was missing.

@Shnoulle
Copy link
Collaborator

OK ! Tested OK

OK, the issue was the minified file was missing.

debug=0 issue … didn't happen if debug set … ouf …

@twilligls
Copy link
Contributor

works for me as well!

@olleharstedt olleharstedt merged commit fa69364 into master Jan 5, 2023
@c-schmitz c-schmitz deleted the bug/18543--Question-Type-Array--texts--crashes-when-in-combination-with-Array-filter-style-disabled-and-show-totals branch June 26, 2023 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code review done Version checked for code issue without testing Tested OK This PR has been tested by QA and works as expected
Projects
None yet
5 participants