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

[A11y] Add label for RecentlyPlayedActivity [Blocked on #3095] #2824

Closed
rt4914 opened this issue Mar 2, 2021 · 8 comments · Fixed by #4511
Closed

[A11y] Add label for RecentlyPlayedActivity [Blocked on #3095] #2824

rt4914 opened this issue Mar 2, 2021 · 8 comments · Fixed by #4511
Assignees
Labels
Impact: Low Low perceived user impact (e.g. edge cases). 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 Mar 2, 2021

Task

Introduce label for RecentlyPlayedActivity as either Stories for you or Recently played stories.

Reference PR: #2750

When you create a PR for this, make sure you add test to check the functionality. Also, suggest working on this PR only if its your first or second PR to Oppia-Android project.

General Explanation

When Talkback is on, whenever we click on any option which leads to new screen the reader will automatically read the label associated with that screen first. If in case label in not present then it will read app name i.e. Oppia.

Example

device-2021-02-24-021647.mp4

In this above video as soon as we double-tap Battery, new screen is opened.
The reader first outputs Battery followed by Navigate up, Button and finally Battery again. In this the first Battery output is the label of this new screen which indicates the learner which screen is opened. The second Battery is because we selected the title in the screen and it's just reading that text.
So we need labels for various screens general which matches the title but if the title is dynamic then we need static label.

@rt4914 rt4914 added Priority: Essential This work item must be completed for its milestone. good first issue This item is good for new contributors to make their pull request. Type: Task A single task of work corresponding to a greater milestone. Generally corresponds to a single PR. labels Mar 2, 2021
@rt4914 rt4914 added this to the Beta milestone Mar 2, 2021
@IAmJaishree
Copy link

@rt4914 I would like to work on it, could you please assign it to me?

@rt4914
Copy link
Contributor Author

rt4914 commented Mar 15, 2021

@IAmJaishree I have assigned you two similar issues. Also I will be working on this as it involves some discussion with teammates. Thanks.

@rt4914 rt4914 self-assigned this Mar 15, 2021
@rt4914 rt4914 changed the title [A11y] Add label for RecentlyPlayedActivity [Blocked on #2803] [A11y] Add label for RecentlyPlayedActivity Apr 9, 2021
@rt4914 rt4914 removed their assignment Jan 18, 2022
@shankarpriyank
Copy link
Contributor

I would like to work on this could you please assign this to me @bkaur-bkj

@bkaur-bkj
Copy link
Contributor

@rt4914 can this issue be assigned to @shankarpriyank ?

@rt4914 rt4914 removed the good first issue This item is good for new contributors to make their pull request. label Feb 2, 2022
@rt4914
Copy link
Contributor Author

rt4914 commented Feb 2, 2022

@bkaur-bkj Actually no. I just removed good-first-issue label from it.

@bkaur-bkj
Copy link
Contributor

@bkaur-bkj Actually no. I just removed good-first-issue label from it.

okay

@rishidyno
Copy link
Contributor

rishidyno commented Feb 10, 2022

@rt4914 @BenHenning can I work on this issue, as I am quite interested?

@rt4914
Copy link
Contributor Author

rt4914 commented Feb 14, 2022

@rishidyno Suggest doing following things:

  1. Play with current app to see what is the behaviour of screen-reader on this screen under various cases where title changes for the toolbar.
  2. Check this PR Fixes #2824: [A11y] Add label for RecentlyPlayedActivity #3065
  3. Once you have understand the problem and the PR that I had worked on, please mention the approach that you will be following to solve this issue.

Actually this issue is not like any other Add Label issue. Its a bit complicated and will need changes mainly in how we work around intents.

@rishidyno rishidyno self-assigned this Feb 15, 2022
@rt4914 rt4914 changed the title [A11y] Add label for RecentlyPlayedActivity [A11y] Add label for RecentlyPlayedActivity [Blocked on #3095] Feb 15, 2022
@rishidyno rishidyno removed their assignment Jun 28, 2022
@vrajdesai78 vrajdesai78 self-assigned this Jul 23, 2022
@BenHenning BenHenning added Impact: Low Low perceived user impact (e.g. edge cases). Z-ibt Temporary label for Ben to keep track of issues he's triaged. issue_user_learner labels Sep 15, 2022
@BenHenning BenHenning removed this from the Beta milestone Sep 16, 2022
BenHenning added a commit that referenced this issue Oct 5, 2022
…tivity (#4511)

* RecentlyPlayedActivityParams proto created with ActivityRouter

* Nit changes

* Optimize imports

* Shifted applicationComponent to developerApplicationComponent

* Optimized import

* ActivityRouterModule updated

* Updated RECENTLY_PLAYED_ACTIVITY_INTENT_EXTRAS_KEY in ProfileProgressFragmentTest

* Static checks fixed

* setTitle added in ReceltyPlayedActivityPresenter

* bazel added and set title to toolbar

* removed unused import

* nit changes

* test updated and made changes in bazel

* dependencies added BUILD.bazel

* static and lint checks fixed

* protobuf lint issue fixed

* Revert "protobuf lint issue fixed"

This reverts commit 8b0c707.

* protofub lint test fixed

* UNSPECIFIED enum renamed

* order changed in bazel

* src of bazel fixed

* dagger deps added in bazel

* KDoc updated for bazel

* nit changes

* Revert "nit changes"

This reverts commit 00c426a.

* Updated kdoc and optimized code

* klint issue fixed

* reformated route/BUILD.bazel

* regex pattern checks fixed

* optimized import

* Nit changes

* Test updated and RecentlyPlayedActivityTitle used

* Optimized import

* Added newline EOF

* ActivityRouter added to bazel

* reformat bazel

* corrected path

* made nit changes

* Nit changes

* added new line

* New approach implemented for dagger

* code optimized

* Fix broken builds for #4511.

* Fixed intentname failing tests

* Imports optimized

* Added tests for ActivityRouter and ActivityRouterModule

* optimized code

* Added KDoc and ActivityRouter in bazel

* fixed typo

* removed practicetab

* ActivityRouterModule added in BottomSheetOptionsMenu

* Optimized imports

* Updated failing tests

* Added activityRouterModule in Intrumentation

* removed activity_router_module from bazel in Instrumentation

* removed activityRouterModule from LogReportingModuleTest

* removed activity_component_factory from bazel in route/BUILD.bazel

* Nit changes

* Optimized import

* Tests for ActivityRouter and ActivityRouterModule updated

* Optimized code for null safety in ProfileProgressHeaderViewModel

* Optimized code

* Updated KDoc

* Updated kDoc

* Nit changes

* Added new tests and updated ActivityRouter to fix failing CI

* Update app/src/sharedTest/java/org/oppia/android/app/profileprogress/ProfileProgressFragmentTest.kt

Co-authored-by: Ben Henning <[email protected]>

* Revert "Update app/src/sharedTest/java/org/oppia/android/app/profileprogress/ProfileProgressFragmentTest.kt"

This reverts commit 3c141fb.

* Updated testname

* Updated logic in ActivityRouter

Co-authored-by: Ben Henning <[email protected]>
Co-authored-by: Ben Henning <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Impact: Low Low perceived user impact (e.g. edge cases). 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.
9 participants