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

[A11y] Reading Text Size #2628

Closed
2 tasks
rt4914 opened this issue Feb 5, 2021 · 11 comments · Fixed by #2929
Closed
2 tasks

[A11y] Reading Text Size #2628

rt4914 opened this issue Feb 5, 2021 · 11 comments · Fixed by #2929
Assignees
@Arjupta
Labels
Priority: Essential This work item must be completed for its milestone. Type: Task A single task of work corresponding to a greater milestone. Generally corresponds to a single PR. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@rt4914
Copy link
Contributor

rt4914 commented Feb 5, 2021 •
edited
Loading

Current Output

current_reading_text_size.mp4

Issues Identified

  • The talkback reads this screen as Oppia at start, instead it should be Reading Text Size
  • Seekbar should be replaced with simple options list like: "Small", "Medium", "Large", "Extra Large". Mock

Note: To understand the above issue(s) completely, it is recommended that you setup Talkback and play with the app keeping it on and that will give you better context.

Accessibility Guide: https://github.com/oppia/oppia-android/wiki/Accessibility-(A11y)-Guide
@rt4914 rt4914 added Status: Blocked Priority: Essential This work item must be completed for its milestone. Type: Task A single task of work corresponding to a greater milestone. Generally corresponds to a single PR. labels Feb 5, 2021
@rt4914 rt4914 added this to the Beta milestone Feb 5, 2021
@Arjupta
Copy link
Contributor

Arjupta commented Feb 5, 2021

@rt4914 while going through this screen with the help of talkback it is very hard to understand the correct purpose of seekbar so it is better to give four variants of textsize in a group of radio button.

@mschanteltc
Copy link

I agree that it would be better to label each option as a word description like "Small," Medium," "Large," and "Extra Large." Numbers/Percentage might skew the user's perception on how it affects the text size.

@Arjupta
Copy link
Contributor

Arjupta commented Feb 7, 2021

@rt4914 doing this would include changes in both layout and kotlin files. Can I work on this? will be needing some guidance though.

@rt4914
Copy link
Contributor Author

rt4914 commented Feb 7, 2021

@mschanteltc can you prepare mocks for this case so that it's easier for Arjun to work on it ?

@rt4914
Copy link
Contributor Author

rt4914 commented Feb 7, 2021

@Arjupta assigned to you but wait for Chantel's mocks.

@mschanteltc
Copy link

Let's show the options as radio buttons, with Small as default. Each selection's font size should be reflected in their respective options so users have a visual reference for which size suits them best. WDYT? (mock)

OP (Reading Display Size)

@Arjupta
Copy link
Contributor

Arjupta commented Feb 14, 2021

Let's show the options as radio buttons, with Small as default. Each selection's font size should be reflected in their respective options so users have a visual reference for which size suits them best. WDYT? (mock)

OP (Reading Display Size)

Looks good to me

@rt4914
Copy link
Contributor Author

rt4914 commented Feb 15, 2021

Let's show the options as radio buttons, with Small as default. Each selection's font size should be reflected in their respective options so users have a visual reference for which size suits them best. WDYT? (mock)

OP (Reading Display Size)

@mschanteltc This mock is really nice and to the point. Thanks.

@rt4914 rt4914 changed the title [A11y] Reading Text Size [Blocked] [A11y] Reading Text Size Feb 15, 2021
@Sparsh1212
Copy link
Contributor

@rt4914 Can I work on this issue, if @Arjupta is not working currently on this.

@rt4914
Copy link
Contributor Author

rt4914 commented Mar 10, 2021

@Arjupta Have a look and if you are not working assign it to @Sparsh1212

@Arjupta
Copy link
Contributor

Arjupta commented Mar 11, 2021

@Arjupta Have a look and if you are not working assign it to @Sparsh1212

@rt4914 pardon for the delay I will be resolving this issue by the end of this week, hope this doesn't block your work.

Arjupta added a commit to Arjupta/oppia-android that referenced this issue Mar 16, 2021
@Arjupta
Fixes oppia#2628: Reading Text Size[A11y]
efacdc5 
Fixes oppia#2628 based on the mock provided here oppia#2628 (comment)
@Arjupta Arjupta mentioned this issue Mar 16, 2021
Merged
8 tasks
rt4914 pushed a commit that referenced this issue Jun 9, 2021
@Arjupta @anandwana001
Fixes #2628: Reading Text Size[A11y] (#2929)
82d8ee4 
* Fixes #2628: Reading Text Size[A11y]

Fixes #2628 based on the mock provided here #2628 (comment)

* EOF fix

* Lint Checks

* Corrected the Talkback Output

* Included TextSizeItemViewModel in BUILD.bazel

* Included ReadingTextSizeSelectionViewModel in BUILD.bazel

* Included TextSizeRadioButtonListener in BUILD.bazel

* Changes for RedingTextSizeSelectionViewModel which has resource imports

* Fixing the tests for New TextSizeSelection UI

* Fixing Lint Issues

* Fixed the TextSize ViewMatcher

* Added KDocs and refactored function names

* Apply suggestions from code review

Co-authored-by: Akshay Nandwana <[email protected]>

* Corrections due to applied suggestions from Code Review

* Reverting one suggestion of not using lazy initialization

* Nit changes

* Fixes Bug of not updating reading text size correctly

* Changed the location of ReadingTextSizeFragmentTest

* add the file

* Suggested changes

* Sugggested changes by @rt4914

* Failing check due to unwanted import

* added kdoc for ReadingTextSizeFragment

* Added padding dimension for landscape layout and removed val from injected fragment

* Suggested update

Co-authored-by: Akshay Nandwana <[email protected]>

* Adding qualifier as suggested

* Adding suggested comments

* Corrected the comment format

Co-authored-by: Akshay Nandwana <[email protected]>
@BenHenning BenHenning added the Z-ibt Temporary label for Ben to keep track of issues he's triaged. label Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Arjupta Arjupta

Labels
Priority: Essential This work item must be completed for its milestone. Type: Task A single task of work corresponding to a greater milestone. Generally corresponds to a single PR. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
[Team] Core Learner and Mastery flows & UI Frontend - Android
Archived in project
[Task Force] Full accessibility support - Android
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes #2628: Reading Text Size[A11y] Arjupta/oppia-android
5 participants
@rt4914 @BenHenning @mschanteltc @Sparsh1212 @Arjupta