ground-android icon indicating copy to clipboard operation
ground-android copied to clipboard

Disabling the "Other" Option Now Disables the Other Text and Hides the Keyboard

Open NudurupatiSurya opened this issue 1 year ago • 13 comments

Fixes #2399

Demo:

https://github.com/google/ground-android/assets/53263580/90463bc9-5cb7-439a-8bd8-56c77938d692

@... PTAL?

NudurupatiSurya avatar Jun 28 '24 19:06 NudurupatiSurya

@NudurupatiSurya I believe to test this scenario you would want to add a UI test to MultipleChoiceTaskFragmentTest, since a unit test on MultipleChoiceTaskViewModel might be too trivial. @anandwana001 to confirm.

gino-m avatar Jul 01 '24 07:07 gino-m

@NudurupatiSurya I believe to test this scenario you would want to add a UI test to MultipleChoiceTaskFragmentTest, since a unit test on MultipleChoiceTaskViewModel might be too trivial. @anandwana001 to confirm.

Yes, We do have a file MultipleChoiceTaskFragmentTest. @NudurupatiSurya You can once check the MultipleChoiceTaskFragmentTest and confirm if you can write a similar test to confirm this Other option disable thing.

anandwana001 avatar Jul 02 '24 03:07 anandwana001

Hi @NudurupatiSurya Feel free to ask any question to make this PR to its completion. Thanks

anandwana001 avatar Jul 08 '24 06:07 anandwana001

Gentle Reminder @NudurupatiSurya

anandwana001 avatar Jul 19 '24 10:07 anandwana001

MultipleChoiceTaskFragmentTest Hi, Sorry for the delay. Thank you for your patience and feedback. I will update the PR by tomorrow.

NudurupatiSurya avatar Jul 21 '24 03:07 NudurupatiSurya

Hi,

I wanted to give you a quick update. Before adding the new tests, I ran the existing ones in MultipleChoiceTaskFragmentTest.kt and found that the following tests are failing due to the recent changes:

  • selects other option on text input and deselects other radio inputs
  • no deselection of other option on text clear when not required
  • no deselection of non-other selection when other is cleared
  • deselects other option on text clear and required

I am currently working on resolving this and adding the new tests as suggested. I'll reach out if I get stuck anywhere. I should have an update for you later this week.

Thanks!

NudurupatiSurya avatar Jul 29 '24 00:07 NudurupatiSurya

Hi @NudurupatiSurya

All the tests are passing for me. Could you please tell me how you are running the test cases? also, is your branch up-to-date?

Screenshot 2024-08-01 at 3 29 18 PM

anandwana001 avatar Aug 01 '24 10:08 anandwana001

Hi @NudurupatiSurya Gentle Reminder

anandwana001 avatar Aug 06 '24 06:08 anandwana001

@NudurupatiSurya Updated branch here and rerunning test on CI to keep things moving foward.

/gcbrun

gino-m avatar Aug 07 '24 14:08 gino-m

@anandwana001 Confirmed tests are failing with the following when merged with HEAD:


com.google.android.ground.ui.datacollection.tasks.multiplechoice.MultipleChoiceTaskFragmentTest > selects other option on text input and deselects other radio inputs FAILED
    androidx.test.espresso.PerformException at MultipleChoiceTaskFragmentTest.kt:176
        Caused by: java.lang.RuntimeException at MultipleChoiceTaskFragmentTest.kt:176

com.google.android.ground.ui.datacollection.tasks.multiplechoice.MultipleChoiceTaskFragmentTest > no deselection of other option on text clear when not required FAILED
    androidx.test.espresso.PerformException at MultipleChoiceTaskFragmentTest.kt:210
        Caused by: java.lang.RuntimeException at MultipleChoiceTaskFragmentTest.kt:210

com.google.android.ground.ui.datacollection.tasks.multiplechoice.MultipleChoiceTaskFragmentTest > no deselection of non-other selection when other is cleared FAILED
    androidx.test.espresso.PerformException at MultipleChoiceTaskFragmentTest.kt:227
        Caused by: java.lang.RuntimeException at MultipleChoiceTaskFragmentTest.kt:227

com.google.android.ground.ui.datacollection.tasks.multiplechoice.MultipleChoiceTaskFragmentTest > deselects other option on text clear and required FAILED
    androidx.test.espresso.PerformException at MultipleChoiceTaskFragmentTest.kt:190
        Caused by: java.lang.RuntimeException at MultipleChoiceTaskFragmentTest.kt:190

@NudurupatiSurya Any updates?

gino-m avatar Aug 07 '24 15:08 gino-m

ok,

I have the latest update, and tests are passing for me. Let me try again and get back.

anandwana001 avatar Aug 07 '24 16:08 anandwana001

Hi @gino-m & @anandwana001,

I am currently in the process of some job interviews, so I couldn't reply back promptly.

@anandwana001, even after updating my current branch, these tests are failing

I think the tests are failing because the condition I added is overriding some of the otherText's previous behavior of selecting the other radio button when the user selects otherText.

All the failed tests are trying to write something in otherText before selecting the radio button. I removed the condition I wrote and these tests passed.

I will update my logic so that these tests will not fail and otherText shows the expected behavior. I will update this by next week as I am currently busy with job interviews.

Thank you for your understanding.

NudurupatiSurya avatar Aug 08 '24 15:08 NudurupatiSurya

Hi @NudurupatiSurya any updates?

jcqli avatar Aug 22 '24 19:08 jcqli

@NudurupatiSurya Are you able to finalize this PR within the next week or so? The following tests are failing:

> Task :ground:testLocalDebugUnitTest

com.google.android.ground.ui.datacollection.tasks.multiplechoice.MultipleChoiceTaskFragmentTest > selects other option on text input and deselects other radio inputs FAILED
    androidx.test.espresso.PerformException at MultipleChoiceTaskFragmentTest.kt:177
        Caused by: java.lang.RuntimeException at MultipleChoiceTaskFragmentTest.kt:177

com.google.android.ground.ui.datacollection.tasks.multiplechoice.MultipleChoiceTaskFragmentTest > no deselection of other option on text clear when not required FAILED
    androidx.test.espresso.PerformException at MultipleChoiceTaskFragmentTest.kt:211
        Caused by: java.lang.RuntimeException at MultipleChoiceTaskFragmentTest.kt:211

com.google.android.ground.ui.datacollection.tasks.multiplechoice.MultipleChoiceTaskFragmentTest > no deselection of non-other selection when other is cleared FAILED
    androidx.test.espresso.PerformException at MultipleChoiceTaskFragmentTest.kt:[228](https://github.com/google/ground-android/actions/runs/11942397209/job/33289359379?pr=2528#step:5:229)
        Caused by: java.lang.RuntimeException at MultipleChoiceTaskFragmentTest.kt:228

com.google.android.ground.ui.datacollection.tasks.multiplechoice.MultipleChoiceTaskFragmentTest > deselects other option on text clear and required FAILED
    androidx.test.espresso.PerformException at MultipleChoiceTaskFragmentTest.kt:191
        Caused by: java.lang.RuntimeException at MultipleChoiceTaskFragmentTest.kt:191

gino-m avatar Nov 20 '24 22:11 gino-m

I ran locally and verified that tests are still in fact failing, and that the behavior of this PR doesn't match what's being asked in #2399. I'll revise that ticket to make what's required more clear.

gino-m avatar Dec 03 '24 22:12 gino-m