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

Refactor Auto complete widget #1398

Merged

Conversation

aditya-07
Copy link
Collaborator

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

Fixes #978

Description

  1. Removed FlexboxLayout and used material ChipGroup to add Chips for selected answers.
  2. TextInput is always below the Chips (see attached screenshot).
  3. Added Auto Complete as component in the Catalog app.

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)
Auto-Complete

Auto-Complete-landscape

Checklist

  • I have read and acknowledged the Code of conduct.
  • I have read the Contributing page.
  • 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 demo app(s) to verify my change fixes the issue and/or does not break the demo app(s).

@codecov
Copy link

codecov bot commented May 18, 2022

Codecov Report

Merging #1398 (3811ced) into master (087b8a3) will increase coverage by 0.33%.
The diff coverage is 18.75%.

@@             Coverage Diff              @@
##             master    #1398      +/-   ##
============================================
+ Coverage     89.74%   90.08%   +0.33%     
  Complexity      646      646              
============================================
  Files           129      129              
  Lines          9879     9839      -40     
  Branches        742      713      -29     
============================================
- Hits           8866     8863       -3     
+ Misses          622      591      -31     
+ Partials        391      385       -6     
Impacted Files Coverage Δ
.../QuestionnaireItemAutoCompleteViewHolderFactory.kt 39.58% <18.75%> (+10.90%) ⬆️
.../java/com/google/android/fhir/search/MoreSearch.kt 94.49% <0.00%> (-0.92%) ⬇️

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 e331b97...3811ced. Read the comment docs.

@PallaviGanorkar
Copy link
Contributor

PallaviGanorkar commented May 18, 2022

Doing first level of review

After removing default field "Asthma", Autocomplete View show error symbol without any error msg and also user is able to click submit with error on the screen. Details are in screenshot

Screenshot_20220518_210106

@aditya-07
Copy link
Collaborator Author

Doing first level of review

After removing default field "Asthma", Autocomplete View show error symbol without any error msg and also user is able to click submit with error on the screen. Details are in screenshot

Screenshot_20220518_210106

Fixed the issue with showing error.
Auto-Complete_error

@KhumboLihonga
Copy link

Here are my thoughts:

  1. When typing, I think it would be better if the suggestions would show after typing the first letter as opposed to the first two letters.
  2. If repeats is set to false i.e. only one option is allowed, after the option is selected, the widget should not expand to a multi line widget. It should remain on a single line.
  3. If repeats is set to false i.e. only one option is allowed, after the option is selected, the blinking cursor should disappear. If a user wants to replace the existing selection, they can tap the 'x' on the chip to remove it. Once it is removed, the blinking cursor can re-appear to signal they can add a selection once again.

@aditya-07
Copy link
Collaborator Author

@jingtang10 @shelaghm

Here are my thoughts:

  1. When typing, I think it would be better if the suggestions would show after typing the first letter as opposed to the first two letters.
  2. If repeats is set to false i.e. only one option is allowed, after the option is selected, the widget should not expand to a multi line widget. It should remain on a single line.
  3. If repeats is set to false i.e. only one option is allowed, after the option is selected, the blinking cursor should disappear. If a user wants to replace the existing selection, they can tap the 'x' on the chip to remove it. Once it is removed, the blinking cursor can re-appear to signal they can add a selection once again.

@shelaghm
Copy link

@KhumboLihonga These recommendations sound good to me from a UX perspective.
@jingtang10 what do you think?

@jingtang10 @shelaghm

Here are my thoughts:

  1. When typing, I think it would be better if the suggestions would show after typing the first letter as opposed to the first two letters.
  2. If repeats is set to false i.e. only one option is allowed, after the option is selected, the widget should not expand to a multi line widget. It should remain on a single line.
  3. If repeats is set to false i.e. only one option is allowed, after the option is selected, the blinking cursor should disappear. If a user wants to replace the existing selection, they can tap the 'x' on the chip to remove it. Once it is removed, the blinking cursor can re-appear to signal they can add a selection once again.

@jingtang10
Copy link
Collaborator

@aditya-07 can you please resolve the conflicts? thanks!

@jingtang10 jingtang10 enabled auto-merge (squash) June 21, 2022 17:24
@Tarun-Bhardwaj Tarun-Bhardwaj added the P1 High priority issue label Jun 27, 2022
@jingtang10 jingtang10 merged commit be98cd5 into google:master Jun 29, 2022
@aditya-07 aditya-07 deleted the ak/978-auto-complete-widget-rewrite branch April 19, 2023 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 High priority issue
Projects
Archived in project
6 participants