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

Stats accessibility improvements 1 #10274

Merged
merged 10 commits into from Jul 26, 2019

Conversation

@planarvoid
Copy link
Contributor

commented Jul 24, 2019

This PR improves accessibility of the Overview card and of the Quick scan items.

  • The Value and the Change in the Overview card should now have a natural content description.
  • The Graph has a content description
  • The clickable columns below the graph now have a better content description and an announcement when the user clicks them
  • The Quick scan items are now read in order
  • The graph and the legend are not marked as "not important for accessibility"

I've created this PR from another one that grew too much so there is a chance I missed something related.

Overview card:
Screenshot 2019-07-24 at 10 07 41

Quick scan item:
Screenshot 2019-07-24 at 09 48 47

To test:

  • Test the two cards in the Talkback mode to hear if the content description works as expected

@planarvoid planarvoid added this to the 13.0 milestone Jul 24, 2019

@planarvoid planarvoid requested a review from malinajirka Jul 24, 2019

@planarvoid planarvoid self-assigned this Jul 24, 2019

@peril-wordpress-mobile

This comment has been minimized.

Copy link

commented Jul 24, 2019

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

@malinajirka
Copy link
Contributor

left a comment

Thank you @planarvoid!

I've tested the Overview and QuickScan cards and they work as intended;)

LGTM! 🚢

SideNote: It'd be great to add "ExploreByTouch" support to the GraphView. The previous version of stats had this support and the UX was much better. However, it's out of the scope of this PR.

<string name="stats_bar_chart_content_description">Graph of site views.</string>
<string name="stats_overview_content_description">%1$s %2$s for period: %3$s, change from previous period - %4$s</string>
<string name="stats_graph_updated">Graph updated.</string>
<string name="stats_list_item_short_description">%1$s: %2$s</string>

This comment has been minimized.

Copy link
@malinajirka

malinajirka Jul 26, 2019

Contributor

I'd consider adding "translatable=false" attribute and taking care of RtL written languages in the code.

@planarvoid

This comment has been minimized.

Copy link
Contributor Author

commented Jul 26, 2019

@malinajirka thanks for the check 👍 it should be ready for another check. I'll create a ticket to explore the options of ExploreByTouch - #10285

@malinajirka

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2019

LGTM, thanks! :)

@malinajirka malinajirka merged commit 3a3467b into develop Jul 26, 2019

4 checks passed

Peril Found some issues. Don't worry, everything is fixable.
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: strings-check Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.