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

Talkback does not read the expected text #4739

Closed
JishnuGoyal opened this issue Nov 17, 2022 · 29 comments · Fixed by #5152
Closed

Talkback does not read the expected text #4739

JishnuGoyal opened this issue Nov 17, 2022 · 29 comments · Fixed by #5152
Assignees
Labels
enhancement End user-perceivable enhancements. Impact: Low Low perceived user impact (e.g. edge cases). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Work: Low Solution is clear and broken into good-first-issue-sized chunks.

Comments

@JishnuGoyal
Copy link
Contributor

JishnuGoyal commented Nov 17, 2022

Describe the bug

On the enter pin page on a user profile, the talkback directly reads the enter pin text. Expectation: It should start from Hi, (user name), Please enter your pin.

To Reproduce
Steps to reproduce the behavior:

  1. Go to a user profile which has a pin with talkback turned on
  2. Talkback directly is focussed on text input field and reads "Pin verification".

Environment

  • Device/emulator being used:
  • Android or SDK version (e.g. Android 5 or SDK 21): Android 12
  • App version (you can get this through system app settings or via the admin controls menu in-app): 0.10-beta-5e64fae55e
@BenHenning
Copy link
Sponsor Member

This seems like an a11y nice-to-have, not a broken flow, so we don't need to block the release.

@rt4914 rt4914 added the Priority: Nice-to-have This work item is nice to have for its milestone. label May 15, 2023
@adhiamboperes adhiamboperes added Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). enhancement End user-perceivable enhancements. Work: Low Solution is clear and broken into good-first-issue-sized chunks. labels Jul 16, 2023
@adhiamboperes adhiamboperes added Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. good first issue This item is good for new contributors to make their pull request. labels Aug 10, 2023
@Vishwajith-Shettigar
Copy link
Collaborator

can i work on this issue ?

@adhiamboperes
Copy link
Collaborator

Hi @Vishwajith-Shettigar, could you work on getting your first PR approved, then you can start on this other issue? Thanks!

@Vishwajith-Shettigar
Copy link
Collaborator

Hi @Vishwajith-Shettigar, could you work on getting your first PR approved, then you can start on this other issue? Thanks!

yeah okay

@adhiamboperes
Copy link
Collaborator

@Vishwajith-Shettigar, could you please begin work on this issue?

@adhiamboperes adhiamboperes added Impact: Low Low perceived user impact (e.g. edge cases). and removed Priority: Nice-to-have This work item is nice to have for its milestone. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). labels Sep 4, 2023
@Vishwajith-Shettigar
Copy link
Collaborator

@Vishwajith-Shettigar, could you please begin work on this issue?

Sure thank you

@Vishwajith-Shettigar
Copy link
Collaborator

Screenshot_2023-09-08-19-36-02-48_943a62cb4c6fb83e010e1c2e82766a17
@adhiamboperes Can you please confirm me issue is addressing the same page in screenshot

@MohitGupta121
Copy link
Member

@Vishwajith-Shettigar correct this is same.

@adhiamboperes
Copy link
Collaborator

@Vishwajith-Shettigar correct this is same.

Thanks @MohitGupta121. @Vishwajith-Shettigar, please let us know if you have any questions about the solution

@Vishwajith-Shettigar
Copy link
Collaborator

Vishwajith-Shettigar commented Sep 8, 2023

@adhiamboperes We are expecting when activity opens talk back should read Hi, (user name), Please enter your pin. then after talk back will focus on Edit text. Am I going right ?

@Vishwajith-Shettigar
Copy link
Collaborator

@Vishwajith-Shettigar correct this is same.

thank you

@adhiamboperes
Copy link
Collaborator

@adhiamboperes We are expecting when activity opens talk back should read Hi, (user name), Please enter your pin. then after talk back will focus on Edit text. Am I going right ?

The user greeting textview needs a content description to be read, same to the enter pin textview

@Vishwajith-Shettigar
Copy link
Collaborator

@adhiamboperes We are expecting when activity opens talk back should read Hi, (user name), Please enter your pin. then after talk back will focus on Edit text. Am I going right ?

The user greeting textview needs a content description to be read, same to the enter pin textview

Talk back reads activity label first that is "pin verification" now what should i do to mute that, am i allowed to remove activity label ?

@adhiamboperes
Copy link
Collaborator

The user greeting textview needs a content description to be read, same to the enter pin textview

Talk back reads activity label first that is "pin verification" now what should i do to mute that, am i allowed to remove activity label ?

What we need is for talkback to read the texts on the screen in the order in which they appear. The way to do this is to make sure the textviews all have corresponding content descriptions. The activity labels shoud not be modified. To clear any confusion caused by the issue description, please just make sure what we have on the screen is what is actually being read.

@Vishwajith-Shettigar
Copy link
Collaborator

The user greeting textview needs a content description to be read, same to the enter pin textview

Talk back reads activity label first that is "pin verification" now what should i do to mute that, am i allowed to remove activity label ?

What we need is for talkback to read the texts on the screen in the order in which they appear. The way to do this is to make sure the textviews all have corresponding content descriptions. The activity labels shoud not be modified. To clear any confusion caused by the issue description, please just make sure what we have on the screen is what is actually being read.

all i just need is to give content description right ? or something else ?

@adhiamboperes
Copy link
Collaborator

Please start there and confirm that the solution works as expected. Afterwards you can also check the corresponding test files and add tests for the new content descriptions. You can find the pattern for testing from existing tests on the same.

@Vishwajith-Shettigar
Copy link
Collaborator

@adhiamboperes i set content description for user greeting textview and enter pin textview, but the issue is talback directly focus on edit text when we enter the activity.

@adhiamboperes
Copy link
Collaborator

adhiamboperes commented Sep 15, 2023

@adhiamboperes i set content description for user greeting textview and enter pin textview, but the issue is talback directly focus on edit text when we enter the activity.

There is an xml attribute android:importantForAccessibility="yes" that tells talkback a view is important. Please research it a bit and then try it.

@Vishwajith-Shettigar
Copy link
Collaborator

@adhiamboperes i set content description for user greeting textview and enter pin textview, but the issue is talback directly focus on edit text when we enter the activity.

There is an xml attribute android:importantForAccessibility="yes" that tells talkback a view is important. Please research it a bit and then try it.

I have tried that, if i make android:importantForAccessibility="no" for edit text that works but edit text wont be able to read by talkback even if user touch it

@Vishwajith-Shettigar
Copy link
Collaborator

@adhiamboperes i set content description for user greeting textview and enter pin textview, but the issue is talback directly focus on edit text when we enter the activity.

There is an xml attribute android:importantForAccessibility="yes" that tells talkback a view is important. Please research it a bit and then try it.

I have tried that, if i make android:importantForAccessibility="no" for edit text that works but edit text wont be able to read by talkback even if user touch it

and i have tried android:importantForAccessibility="yes" for textviews but there is no difference

@adhiamboperes
Copy link
Collaborator

Can you please research this and let me know what you come up with?

@Vishwajith-Shettigar
Copy link
Collaborator

Okay i will try

@Vishwajith-Shettigar
Copy link
Collaborator

I came up with a solution, talkback is directly focusing on edit text because of edit text auto focus by ,making edittext focusable="false" and dynamically setting focusable="true" when user click on edit text, this might solve.

@adhiamboperes
Copy link
Collaborator

That sounds like a great idea. Please go ahead with the implementation.

@Vishwajith-Shettigar
Copy link
Collaborator

That sounds like a great idea. Please go ahead with the implementation.

@adhiamboperes here is PR #5152

@adhiamboperes
Copy link
Collaborator

I have reviewed the talkback experience on this screen---it sounds to me like the text being read is sufficient for the user to know what is required, and I don't think there is need to modify it at this point. Here is a recording. @seanlip , @BenHenning, please review.

Screenrecorder-2023-10-06-17-31-32-803.mp4

@seanlip
Copy link
Member

seanlip commented Oct 6, 2023

@adhiamboperes I'm not sure what the expected behaviour for a11y is when there is autofocus enabled -- @BenHenning might know better here so it might be worth asking him directly.

One thought I had -- will they know that there's a "Forgot PIN?" option on the screen, if we don't enable the whole screen to be read at the outset? I feel like, based on this, I lean towards thinking it should be changed along the lines that @JishnuGoyal proposed, if only for consistency with other screens (and thinking about the case where we might add more stuff to this screen in future).

Just my 2c.

@seanlip
Copy link
Member

seanlip commented Oct 6, 2023

Also to clarify -- my previous comment was meant to describe what I think is the "ideal" case, but I think that fixing this is still probably fairly low impact and that it might be better to work on higher-impact issues instead.

(@adhiamboperes just a note, we probably shouldn't really be putting "good first labels" on low-impact projects in general, if there are other things that would be more important to do. Better to break down the higher-impact issues if possible so that they're approachable.)

@adhiamboperes
Copy link
Collaborator

Thanks @seanlip, I'll keep that in mind.

@adhiamboperes adhiamboperes removed the good first issue This item is good for new contributors to make their pull request. label Oct 6, 2023
adhiamboperes pushed a commit that referenced this issue Oct 19, 2023
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fix #4739, edittext requesting focus only when reader is off will
resolve this issue.
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] 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: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
Before fix


https://github.com/oppia/oppia-android/assets/76042077/41bc96b4-3fe1-4688-a77b-02bfdbd22782


After fix

  Talkback On


https://github.com/oppia/oppia-android/assets/76042077/a01da3dc-8848-4632-a8c0-17ce3e409beb


Talkback Off



https://github.com/oppia/oppia-android/assets/76042077/beac309e-16d8-496f-bc09-af8de634536b





 

<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing
Rd4dev pushed a commit to Rd4dev/oppia-android that referenced this issue Oct 31, 2023
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fix oppia#4739, edittext requesting focus only when reader is off will
resolve this issue.
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] 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: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
Before fix


https://github.com/oppia/oppia-android/assets/76042077/41bc96b4-3fe1-4688-a77b-02bfdbd22782


After fix

  Talkback On


https://github.com/oppia/oppia-android/assets/76042077/a01da3dc-8848-4632-a8c0-17ce3e409beb


Talkback Off



https://github.com/oppia/oppia-android/assets/76042077/beac309e-16d8-496f-bc09-af8de634536b





 

<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing
Vishwajith-Shettigar added a commit to Vishwajith-Shettigar/oppia-android that referenced this issue Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement End user-perceivable enhancements. Impact: Low Low perceived user impact (e.g. edge cases). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Work: Low Solution is clear and broken into good-first-issue-sized chunks.
Development

Successfully merging a pull request may close this issue.

7 participants