Open Choice widget #801

kevinmost · 2021-09-28T23:48:31Z

IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).

Fixes #689

Description
This PR refactors out OptionSelectDialogFragment from the previous MultiSelectDialogFragment. OptionSelectDialogFragment now supports all permutations of the following options:

addOtherOptions: If this is true, the dialog will append an additional option called "Other". When this option is selected, if multiSelectEnabled = false, the user will see a single EditText in which they can enter a free-form option. If multiSelectEnabled = true, the user will see an "Add another answer" button. When clicked, this button will show additional EditTexts (which will have delete buttons next to them).
multiSelectEnabled: This is controlled by the repeats option on a questionnaire item. This decides whether checkboxes or radio buttons are used, and affects the behavior of addOtherOptions as detailed above.

This PR also modifies the behavior of "type": "choice" UX elements when an itemControl is not explicitly specified. Questions with 10+ answers are now presented in a dialog.

Alternative(s) considered
Discussed offline. This approach gives us the most flexibility and conforms best to FHIR data standards.

Type
Choose one: (Feature)

Screenshots (if applicable)

Single-select:

Multi-select:

Questionnaire Response

Note that valueCoding is used for pre-defined answers, while valueString is used for custom answers:

Checklist

I have read and acknowledged the Code of conduct
I have read How to Contribute
I have read the Developer's guide
I have signed the Google Individual CLA, or I am covered by my company's Corporate CLA
I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach
I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the style guide of this project
I have run ./gradlew check and ./gradlew connectedCheck to test my changes locally
I have built and run the reference app(s) to verify my change fixes the issue and/or does not break the reference app(s)

jingtang10

This is exemplary work. Thanks @kevinmost!

One design decision in the single select case: Should "Other" and the edit text box be on the same line? I think there might be space concern.. but if they're not on the same line, do we want to hide the edit text until other is selected? Can we get some UX input?

In the multi select case - do we even need to have an "other" checkbox? Can we just not have it?

Alternatively, we can have an edit text with an "add option" button which if you click will convert what's in the edit text to a checkbox that is automatically ticked (and can't be unticked, but can be removed)...

...apture/src/main/java/com/google/android/fhir/datacapture/views/OptionSelectDialogFragment.kt

shelaghm · 2021-09-29T23:08:27Z

UX perspective

I think we can do it on the same line if there is space (might work in English and larger phone sizes, but I worry about how this will scale across phones/languages). Attached screenshot below of what this can look like. Google forms does something similar to this and I think it works well.
Agree, don't show the enter text field until other is selected.
Re: removing multi-select "other" checkbox: I'm curious to hear more why you propose remove it? I'm not sure I'm fully following, do you mean (a) removing the whole option of "other" in the list of options and replacing it with a button that says "add your own" or just (b) removing the checkbox? My gut reaction is that I think having an other option in the list is maybe more of a common pattern, but not opposed to adding a button instead.
What are your thoughts on the remove icon, do you think the (-) or trash icon?

@kevinmost @jingtang10

jingtang10 · 2021-09-30T07:12:00Z

Thanks @shelaghm!

I don't really think the "other" check box does much in the multi select case. Can the user untick the box and will the user input options still be there but just not selected as answers (discarded when you submit the questionnaire)?

my proposal is have all the given options, and have a button that says add other, which will create a new edit text with a remove button next to it. and you can click the button again to add another row. these rows will be automatically selected. so the pattern is very similar to what's in the existing implementation, just without the other checkbox, and with the edit text and the add button always visible.

another possible design is we don't even have an add button... whenever you start typing in an edit text, another row will be created. this feels super slick :)

To me, in the multi select case at least, the presence of the user input indicates the custom options should be selected. to remove them that's what the remove button is for.

jingtang10 · 2021-09-30T17:43:32Z

re 4 i like "-" because it's simple and clear. the trash icon looks very busy and seems highly contextual... (maybe to some people trash cans don't look exactly like that?)

shelaghm · 2021-10-04T16:48:54Z

@jingtang10

Thanks for elaborating. I hear you on that it seems more clear to use the remove '-' button than to uncheck the other box if you want to unselect/remove the other choice. The 'add other' button could also help with people typing in the text, rather than just selecting other without adding the descriptive text. I mocked up what it could look like with an 'add other' button and without "other" listed in the list of options and I think the pattern works. see below

Is there a scenario where we want to enable selecting other, but not adding edit text?

kevinmost · 2021-10-06T23:25:18Z

I made #816 to track Shelagh's UX suggestions; I fully agree with these changes but feel like we should do them in a follow-up PR.

PTAL @jingtang10, thanks!

codecov-commenter · 2021-10-07T18:57:37Z

Codecov Report

Merging #801 (9d5cf8a) into master (7e79f73) will decrease coverage by 1.10%.
The diff coverage is 19.18%.

@@             Coverage Diff              @@
##             master     #801      +/-   ##
============================================
- Coverage     36.19%   35.08%   -1.11%     
+ Complexity      269      238      -31     
============================================
  Files           117      111       -6     
  Lines          4045     3181     -864     
  Branches        664      562     -102     
============================================
- Hits           1464     1116     -348     
+ Misses         2260     1821     -439     
+ Partials        321      244      -77

Impacted Files	Coverage Δ
...hir/datacapture/MoreQuestionnaireItemComponents.kt	`3.84% <0.00%> (ø)`
...droid/fhir/datacapture/QuestionnaireItemAdapter.kt	`0.00% <0.00%> (ø)`
...hir/datacapture/QuestionnaireItemViewHolderType.kt	`0.00% <0.00%> (ø)`
...ir/datacapture/views/OptionSelectDialogFragment.kt	`0.00% <0.00%> (ø)`
.../QuestionnaireItemDialogSelectViewHolderFactory.kt	`64.19% <64.19%> (ø)`
...gle/android/fhir/db/impl/dao/ResourceDao_Impl.java
...ure/gallery/QuestionnaireListFragmentDirections.kt
...droid/fhir/db/impl/dao/SyncedResourceDao_Impl.java
...le/android/fhir/db/impl/ResourceDatabase_Impl.java
.../android/fhir/db/impl/dao/LocalChangeDao_Impl.java
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e79f73...9d5cf8a. Read the comment docs.

jingtang10

Thanks @kevinmost :)

santosh-pingle · 2023-07-17T08:45:35Z

...apture/src/main/java/com/google/android/fhir/datacapture/views/OptionSelectDialogFragment.kt

+    val otherOptionsAllowed: Boolean,
+  )
+
+  private val viewModel: QuestionnaireItemDialogSelectViewModel by activityViewModels()


@kevinmost @jingtang10
Is the reason we instantiated QuestionnaireItemDialogSelectViewModel using activityViewModels() because it is also shared by QuestionnaireItemDialogSelectViewHolderFactory.kt? The only drawback I see is that even though the QuestionnaireFragment (which is used to render the QuestionnaireViewItem) is not alive, the model is kept in memory, which raises issue #2055

Approach 1 to fix the issue :
private fun selectedOptionsFlow(linkId: String) = linkIdsToSelectedOptionsFlow.getOrPut(linkId) { MutableSharedFlow(replay = 1) }
Is it okay to change the replay count to 0? If the replay count is set to 0, I do not see the issue existing. The selected option is also stored as an answer, so if the same view is rendered, it can be shown as selected.

santosh-pingle · 2023-07-17T08:46:10Z

.../com/google/android/fhir/datacapture/views/QuestionnaireItemDialogSelectViewHolderFactory.kt

+          requireNotNull(holder.question.context.tryUnwrapContext()) {
+            "Can only use dialog select in an AppCompatActivity context"
+          }
+        val viewModel: QuestionnaireItemDialogSelectViewModel by activity.viewModels()


please refer earlier comment.

kevinmost force-pushed the km/open-choice branch 2 times, most recently from 4063d8f to 9bcb787 Compare September 29, 2021 00:08

kevinmost marked this pull request as ready for review September 29, 2021 00:10

kevinmost requested review from aditya-07, deepankarb, jingtang10, joiskash, pankajkatoch, stevenckngaa and yigit as code owners September 29, 2021 00:10

kevinmost mentioned this pull request Sep 29, 2021

SDC MultiSelect widget now supports "single-select" options #789

Closed

8 tasks

jingtang10 reviewed Sep 29, 2021

View reviewed changes

Tarun-Bhardwaj added the Q3 2021 label Oct 6, 2021

kevinmost mentioned this pull request Oct 6, 2021

UX improvements for open-choice widget #816

Open

kevinmost added 2 commits October 6, 2021 19:21

Refactor OptionSelectDialogFragment out of multi-select

a713376

PR comment updates

daf2736

kevinmost force-pushed the km/open-choice branch from 9bcb787 to daf2736 Compare October 6, 2021 23:23

kevinmost enabled auto-merge (squash) October 6, 2021 23:25

kevinmost added 2 commits October 7, 2021 10:48

spotless apply

add738d

fixes for broken tests

9d5cf8a

jingtang10 approved these changes Oct 8, 2021

View reviewed changes

kevinmost merged commit a48881c into master Oct 8, 2021

kevinmost deleted the km/open-choice branch October 8, 2021 15:47

ndegwamartin mentioned this pull request Nov 18, 2021

SDC library does not render valueSet widgets on the Questionnaire #928

Closed

santosh-pingle reviewed Jul 17, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Open Choice widget #801

Open Choice widget #801

kevinmost commented Sep 28, 2021 •

edited

Loading

jingtang10 left a comment

shelaghm commented Sep 29, 2021 •

edited

Loading

jingtang10 commented Sep 30, 2021 •

edited

Loading

jingtang10 commented Sep 30, 2021

shelaghm commented Oct 4, 2021

kevinmost commented Oct 6, 2021

codecov-commenter commented Oct 7, 2021

jingtang10 left a comment

santosh-pingle Jul 17, 2023 •

edited

Loading

santosh-pingle Jul 17, 2023

Open Choice widget #801

Open Choice widget #801

Conversation

kevinmost commented Sep 28, 2021 • edited Loading

Single-select:

Multi-select:

Questionnaire Response

jingtang10 left a comment

Choose a reason for hiding this comment

shelaghm commented Sep 29, 2021 • edited Loading

jingtang10 commented Sep 30, 2021 • edited Loading

jingtang10 commented Sep 30, 2021

shelaghm commented Oct 4, 2021

kevinmost commented Oct 6, 2021

codecov-commenter commented Oct 7, 2021

Codecov Report

jingtang10 left a comment

Choose a reason for hiding this comment

santosh-pingle Jul 17, 2023 • edited Loading

Choose a reason for hiding this comment

santosh-pingle Jul 17, 2023

Choose a reason for hiding this comment

kevinmost commented Sep 28, 2021 •

edited

Loading

shelaghm commented Sep 29, 2021 •

edited

Loading

jingtang10 commented Sep 30, 2021 •

edited

Loading

santosh-pingle Jul 17, 2023 •

edited

Loading