-
Notifications
You must be signed in to change notification settings - Fork 306
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 to prevent checkbox crashes#2661 #2800
base: master
Are you sure you want to change the base?
Conversation
"repeats": true, | ||
"repeats": false, |
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.
this should be true
, because this json example is intended to showcase checkbox widget
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.
@mmanojkmr this is also true.
with repeats set to false, we are not checking multi-choice anymore.
repeats needs to be true.
Given that, you probably need to add the check-box extension (if not already there) like the json in the issue this PR is solving to confirm that SDC is not crashing.
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.
Please add a test case in CheckBoxGroupViewHolderFactoryTest or update in case there is already something testing the issue being fixed here.
if(!questionnaireViewItem.questionnaireItem.repeats){ | ||
newAnswers.clear() | ||
} |
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.
looks a bit odd place to add this logic - you are adding some item to newAnswers by clearing it ?
can you move this piece of code below - after getting newAnswers
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
Can you run |
video.mp4I have added video for functionality. |
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Fixes #2661
Description
fixed to prevent checkbox crashed.
I have cleared the checkbox list if repeat is false.
Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?
Type
Choose one: (Bug fix)
Screenshots (if applicable)
Checklist
./gradlew spotlessApply
and./gradlew spotlessCheck
to check my code follows the style guide of this project../gradlew check
and./gradlew connectedCheck
to test my changes locally.