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.

Sign up for GitHub

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

Jump to bottom

BUG: Fix All warnings in "Accessibility" category #5171

Closed
Tracked by #5169
adhiamboperes opened this issue Oct 2, 2023 · 5 comments · Fixed by #5173
Closed
Tracked by #5169

BUG: Fix All warnings in "Accessibility" category #5171

adhiamboperes opened this issue Oct 2, 2023 · 5 comments · Fixed by #5173
Assignees
@theMr17
Labels
bug End user-perceivable behaviors which are not desirable. good first issue This item is good for new contributors to make their pull request. Impact: Low Low perceived user impact (e.g. edge cases). Work: Low Solution is clear and broken into good-first-issue-sized chunks.
Milestone
1.0 Global availa...

Comments

@adhiamboperes
Copy link
Collaborator

adhiamboperes commented Oct 2, 2023 •
edited

Description

We would like to fix all the remaining lint issues on Oppia Android in "Accessibility" category.

Steps To Reproduce

Run the following command from a terminal in a checked-out Oppia Android repo:

./gradlew :app:lint

Output: All warnings in "Accessibility" category

Under this category, the warnings are mainly of 3 types.

Warning: Image without contentDescription.
Fix: Add a contentDescription to each of those ImageView, or android:importantForAccessibility="no" for views that are safe to ignore.

Warning: 'onTouch' lambda should call 'View#performClick' when a click is detected
Fix: Call customView.performClick( ).

Warning: 'clickable' attribute found, please also add 'focusable'.
Fix: Set android:focusable="true" to allow keyboard navigation per: https://android.googlesource.com/platform/tools/base/+/studio-master-dev/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/KeyboardNavigationDetector.java?autodive=0%2F.

Expected Behavior

Running the command above should not show any errors.
@adhiamboperes adhiamboperes mentioned this issue Oct 2, 2023
Open
25 tasks
@adhiamboperes adhiamboperes changed the title All warnings in "Accessibility" category BUG: Fix All warnings in "Accessibility" category Oct 2, 2023
@adhiamboperes
Copy link
Collaborator Author

@the-mr17, PTAL.

@adhiamboperes adhiamboperes added good first issue This item is good for new contributors to make their pull request. Impact: Low Low perceived user impact (e.g. edge cases). bug End user-perceivable behaviors which are not desirable. Work: Low Solution is clear and broken into good-first-issue-sized chunks. labels Oct 2, 2023
@adhiamboperes adhiamboperes added this to the 1.0 Global availability milestone Oct 2, 2023
@adhiamboperes
Copy link
Collaborator Author

@the-mr17, could you also please fill this out: https://docs.google.com/forms/d/e/1FAIpQLSes4wj7oKtiN0iyUwd-Xt_DAOr1b-MHG9_YYIavWBF4G6g8uA/viewform

@theMr17
Copy link
Collaborator

theMr17 commented Oct 2, 2023 •
edited

@the-mr17, could you also please fill this out: https://docs.google.com/forms/d/e/1FAIpQLSes4wj7oKtiN0iyUwd-Xt_DAOr1b-MHG9_YYIavWBF4G6g8uA/viewform

@adhiamboperes Sure, But I have some queries regarding this:

  1. Here, it asks me to provide links of PRs merged on Oppia Web repo and also mentions about the teams in Oppia Web. Can you please clarify this?

  2. Will it be okay if I am not able to join team meetings, incase I am committed to some other work at that time?

@MohitGupta121
Copy link
Member

MohitGupta121 commented Oct 2, 2023 •
edited

@the-mr17 if you select oppia-android then it give right section.
And be sure you filled this form also before collaborator form. Thanks!

@seanlip
Copy link
Member

seanlip commented Oct 2, 2023

Sorry -- the form was incorrectly configured, if you refresh it it should show you the Android section now.

@theMr17 theMr17 mentioned this issue Oct 2, 2023
Merged
6 tasks
adhiamboperes added a commit that referenced this issue Oct 5, 2023
@theMr17 @adhiamboperes
Fix #5171: Fixes all warnings in "Accessibility" category (#5173)
8aefb05 
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- 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.
  -->

Fixes #5171

Under this category, the warnings are mainly of 3 types:

1. Warning: Image without contentDescription.
Fix: Add a `contentDescription` to each of those `ImageView`, or mark as
not important for accessibility.

2. Warning: 'onTouch' lambda should call 'View#performClick' when a
click is detected
Fix: Call `customView.performClick( )`.

3. Warning: 'clickable' attribute found, please also add 'focusable'.
Fix: Set `android:focusable="true"` as its necessary if
`android:clickable="true"`.


![image](https://github.com/oppia/oppia-android/assets/84731134/894a6fb3-632b-4397-bc80-16b60d06f5f2)

## 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
<!-- 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

---------

Co-authored-by: Adhiambo Peres <59600948+adhiamboperes@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@theMr17 theMr17

Labels
bug End user-perceivable behaviors which are not desirable. good first issue This item is good for new contributors to make their pull request. Impact: Low Low perceived user impact (e.g. edge cases). Work: Low Solution is clear and broken into good-first-issue-sized chunks.
Projects
[Team] Core Learner and Mastery flows & UI Frontend - Android
Archived in project
Milestone
1.0 Global availability
Development

Successfully merging a pull request may close this issue.

Fix #5171: Fixes all warnings in "Accessibility" category theMr17/oppia-android
4 participants
@seanlip @adhiamboperes @MohitGupta121 @theMr17