-
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
Show asterisk in flyover text if question text is not present and questionnaire item is required. #1762
Conversation
@shelaghm |
@santosh-pingle Looks good! Thanks |
@santosh-pingle did we resolve whether we wanted |
...capture/src/main/java/com/google/android/fhir/datacapture/views/MoreQuestionnaireItemView.kt
Outdated
Show resolved
Hide resolved
...capture/src/main/java/com/google/android/fhir/datacapture/views/MoreQuestionnaireItemView.kt
Outdated
Show resolved
Hide resolved
...capture/src/main/java/com/google/android/fhir/datacapture/views/MoreQuestionnaireItemView.kt
Outdated
Show resolved
Hide resolved
...capture/src/main/java/com/google/android/fhir/datacapture/views/MoreQuestionnaireItemView.kt
Outdated
Show resolved
Hide resolved
...capture/src/main/java/com/google/android/fhir/datacapture/views/MoreQuestionnaireItemView.kt
Outdated
Show resolved
Hide resolved
...pture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemHeaderView.kt
Show resolved
Hide resolved
...om/google/android/fhir/datacapture/views/QuestionnaireItemDateTimePickerViewHolderFactory.kt
Outdated
Show resolved
Hide resolved
I will start this thread in issue #544. As per comment from @shelaghm, it is encouraged to use it. |
Thanks @santosh-pingle ! LGTM! |
We will wait for @shelaghm actually to get back to @jingtang10 's comment in the Issue #544 before approving |
@omarismail94 @santosh-pingle I've commented over on #544 let me know if either of you have thoughts as well :) |
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 think we're gonna not use asterisk at all as the result of UX research findings and discussions with @shelaghm.
sorry that you'll need to rework this pr.
@@ -44,6 +44,7 @@ | |||
<string name="not_answered">"Not Answered"</string> | |||
<string name="help">Help</string> | |||
<string name="space_asterisk">\u0020\u002a</string> | |||
<string name="required">\u002arequired</string> |
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.
<string name="required">\u002arequired</string> | |
<string name="required">Required</string> |
Let's wait while we get some input from the community on how they use required and then make a decision on how we re-work this issue. So far we've heard from Ona's designers. Will be great to hear from a few others how they are using this feature in their deployments. |
FYI #544 Ona uses |
@santosh-pingle can we make this configurable? would like to show nothing if required and show |
@santosh-pingle can we close this PR now given it's also out of date? we can reopen/create a new PR once we have conclusion on UX. |
thanks! |
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Fixes #544
Description
Clear and concise code change description.
Alternative(s) considered
Have you considered any alternatives? And if so, why have you chosen the approach in this PR?
Type
Choose one: Feature
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.