Fixes #2627: Improve accessibility for Pin Verification Screen

[vrajdesai78]

Explanation

Fixes #2627: Improve accessibility for Pin Verification screen. I have removed seperate textview which is used to show error text. Now, error is set directly to PinPassPasswordInputEditText using onTextChanged extension function.

Essential Checklist

• The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
• Any changes to scripts/assets files have their rationale included in the PR explanation.
• The PR follows the style guide.
• The PR does not contain any unnecessary code changes from Android Studio (reference).
• The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
• The PR is assigned to the appropriate reviewers (reference).

Before

current_profile_pin_full_video.mp4

After

oppia-pinscreen.mp4

Passing updated testcases

[rt4914]

From Code perspective and implementation its really good.

Not sure why tests are failing, can you run End-to-End and other robolectric tests and check.

Also, maybe just merge with latest develop.

[oppiabot]

Unassigning @rt4914 since the review is done.

[oppiabot]

Hi @vrajdesai78, it looks like some changes were requested on this pull request by @rt4914. PTAL. Thanks!

[rt4914]

Please remove all DisableAccessibilityChecks and corresponding TODO's for #3245

As this PR is fixing all of them too.

[vrajdesai78]

Unable to run robolectric tests locally. I have run espresso tests and they are passing as expected. I have tried to run all the robolectric tests of PinPasswordActivityTest but it's just loading. I have kept tests running for more than 30 mins but still not even one test is getting passed or failed.

[BenHenning]

@vrajdesai78 are you able to run any app module Robolectric tests? If so, does the PinPasswordActivityTest pass for you on Robolectric for the develop branch (i.e. without your changes in this PR)? These details may help provide some clarity into what's happening.

[vrajdesai78]

I have tried with develop branch. All test cases are passing in develop branch.

[rt4914]

I have been trying to check robolectric tests for almost a dozen hours now and these are my findings.

1. testPinPassword_withUser_forgot_inputAdminPinAndNewPinAndOldPin_wrongPinError this test is incorrect as pin_password_incorrect_pin is checked incorrectly.
2. The following four tests are unable to finish on Robolectric:

3. In all these 4 tests if we replace hasErrorText(context.resources.getString(R.string.pin_password_incorrect_pin)) with hasNoErrorText all tests pass which means that there is definitely some issue as the tests are not visible.

4. Also, where-ever we are trying to append the text to the input-pin TextLayoutInput, we should use

 onView(
        allOf(
          withId(R.id.pin_password_input_pin_edit_text),
          isDescendantOfA(withId(R.id.pin_password_input_pin))
        )
      )

and not
onView(withId(R.id.pin_password_input_pin_edit_text))

Try to work around these cases.

Maybe directly add @Ignore to above mentioned 4 tests and run all robolectirc tests without any change and notice they all pass. And now start debugging the remaining 4 tests one by one.

[rt4914]

I have made few changes to test cases (which are incorrect changes too) just to show that all tests are either passing or failing.
The biggest culprit is testCoroutineDispatchers.runCurrent() at some places its blocking the test completely.

Output

Corresponding Code (definitely contains some incorrect code)

https://gist.github.com/rt4914/2982b2a4c0bd13b213959902851040b0

[rt4914]

@BenHenning Can you have a look at my last two comments here. Tests are not finishing in certain cases.

[BenHenning]

@rt4914 how consistently does runCurrent block tests? Have you been able to look at all into what it's doing when blocking?

[BenHenning]

Not stale, I'm still working on this.

[oppiabot]

Hi @vrajdesai78, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

[BenHenning]

I think you can resolve this now @vrajdesai78 by changing pin_password_activity such that it doesn't have nested ConstraintLayouts. That will avoid the problem.

[vrajdesai78]

@BenHenning I have fixed the failing test cases as per you suggestion by setting width to match_parent instead of 0dp. Also, added a new test to check that error is not displayed when correct pin is entered. Thanks

[BenHenning]

Thanks @vrajdesai78. Just had one follow-up comment about the change to pin_password_activity--PTAL.

[BenHenning]

Thanks @vrajdesai78. Just had one follow-up nit otherwise the change LGTM.

[BenHenning]

Thanks @vrajdesai78! LGTM.