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

Fixes part of #2743: Label for Recently Played Activity A11y #2781

Closed
wants to merge 1 commit into from

Conversation

rt4914
Copy link
Contributor

@rt4914 rt4914 commented Feb 25, 2021

Fixes part of #2743

I checked the app and found that the title inside the Recently Played Activity is fixed and therefore just creating this PR.

Should we make this header dynamic similar to the text that we have in Home Screen to the side of View All. The possible options for the header are: Stories for you and Recently played stories.

Reference: https://docs.google.com/document/d/1o7hOz1rP-hJaKn1f5x6LV6ms0bzhyCpsTifJQY5crCc/edit?disco=AAAALj4Y6X8

@rt4914 rt4914 requested a review from BenHenning as a code owner February 25, 2021 11:04
@rt4914
Copy link
Contributor Author

rt4914 commented Feb 25, 2021

The pull request template is not getting used by default now. cc: @BenHenning

Copy link
Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rt4914. Left some thoughts.

@@ -429,4 +429,5 @@
<string name="coming_soon">Coming Soon</string>
<string name="recommended_stories">Recommended Stories</string>
<string name="stories_for_you">Stories For You</string>
<string name="recently_played_activity_label">Recently played stories</string>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we be using title instead of label per the discussion from the team meeting this week?

@@ -41,6 +41,7 @@
android:theme="@style/OppiaThemeWithoutActionBar" />
<activity
android:name=".app.home.recentlyplayed.RecentlyPlayedActivity"
android:label="@string/recently_played_activity_label"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also be using this for the toolbar to ensure the two are kept in sync?

@BenHenning
Copy link
Member

Thanks for confirming @rt4914. Was waiting for someone to test it out. Will roll back #2765.

@rt4914
Copy link
Contributor Author

rt4914 commented Feb 26, 2021

@BenHenning I will update the PR meanwhile please have a look at the PR description and reply on the document.

@rt4914 rt4914 assigned BenHenning and unassigned rt4914 Feb 26, 2021
@BenHenning
Copy link
Member

Responded to the comment. Assigning back to you until this is ready for review.

@BenHenning BenHenning assigned rt4914 and unassigned BenHenning Mar 2, 2021
@rt4914 rt4914 closed this Apr 9, 2021
@rt4914 rt4914 deleted the a11y-recently-played-label branch April 9, 2021 00:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants