-
Notifications
You must be signed in to change notification settings - Fork 295
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
Configurable drop down answer option. #2526
base: master
Are you sure you want to change the base?
Configurable drop down answer option. #2526
Conversation
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 is incorrect as per rules in Questionnaire:-
+ Rule: If one or more answerOption is present, initial cannot be present. Use answerOption.initialSelected instead
However we don't even need this. You can override the hyphen string constant in your application and the dropdown will read that value instead of "-" defined in the sdc library.
What you may do is change the name of the string constant from hyphen
to maybe default_drop_down_value
?
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.
i agree with jajoo - instead of using initial answers, just provide a way for the user to override the string resource in the xml file. what you might want to do is to give that string a better name. and in the same pr please update the documentation in the docs/ folder or sdc library.
val initialAnswerString = | ||
if (questionnaireViewItem.questionnaireItem.hasInitial()) { | ||
questionnaireViewItem.questionnaireItem.initial.first().valueStringType.valueAsString | ||
} else { | ||
context.getString(R.string.hyphen) | ||
} |
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.
the initial answer is not something the ui shoudl get involved with. the questionnaire view model should have already handled this.
initialAnswerString, | ||
initialAnswerString, |
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.
one thing you do want to be careful is that if there's a clash between the user defined default value and one of the answer options from the questionnaire. in this case you might want to have a mechanism to give them distinct names -- this is a very very edge case though and i'm happy for you to just add a todo in the code.
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Fixes #2505
Description
In the DropDownViewHolderFactory the hyphen("-") is being set as the default answer option to the drop down. Instead of setting it a fixed answer I have made it configurable. To change the default answer option the questionnaire item representing the drop down item will have the initial property which will have a valueString. The default answer option will be filled with the initial property value if available else it will be filled hyphen("-").
This approach ensures that the configuration will be done via questionnaire itself and the code need be changed everytime to change a default answer option.
Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?
Type
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.