Accessibility : Posting Activity Stats Improvement #10980

jd-alexander · 2019-12-18T16:48:19Z

Findings

The Posting Stats was inaccessible when using TalkBack. This improvement makes it more accessible for visually impaired users who are navigating the stats functionality.

Solution

The solution was to loop through each Box that represents a day and announce the associated stats. This implementation is similar to what was done on iOS. The Boxes didn't have a content description so refactoring was done to generate it based on the month, day and amount of posts.

Before	After
Before Video	After Video

Testing

Enable TalkBack.
Navigate to the Stats page.
Ensure you are on the Insights Tab.
Select each Month under "Posting Activity" and follow the instructions to hear the announcement.

Tap To Hear Day Stats	Stat Announced

All Stats Announced	No Stats Available

Reviewing

Only 1 reviewer is needed but anyone can review.

Submitter Checklist

I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.
If it's feasible, I have added unit tests.

peril-wordpress-mobile · 2019-12-18T16:57:07Z

You can test the changes on this Pull Request by downloading the APK here.

jd-alexander · 2019-12-18T19:45:02Z

TalkBack is announcing Dec as "Deck" will make the content description of the months the long version.

jd-alexander · 2019-12-19T02:03:14Z

WordPress/src/main/java/org/wordpress/android/ui/stats/refresh/lists/sections/BlockListItem.kt

@@ -233,7 +233,8 @@ sealed class BlockListItem(val type: Type) {
    data class LoadingItem(val loadMore: () -> Unit, val isLoading: Boolean = false) : BlockListItem(LOADING_ITEM)

    data class ActivityItem(val blocks: List<Block>) : BlockListItem(ACTIVITY_ITEM) {
-        data class Block(val label: String, val boxes: List<Box>)
+        data class Block(val label: String, val boxItems: List<BoxItem>)
+        data class BoxItem(val box: Box, val contentDescription: String? = null)


Let me know what you think about the naming here. I am not sure if I am adhering to the naming convention being used so I am rethinking it. So the BoxItem could be renamed to Box and the enum BoxStyle or BoxType

Only the top level items have the Item name. I think this one could be still Box since it's basically a box (from the UI point of view). It's not a list item. I think having a Box and BoxType should be good enough.

Okay thanks @planarvoid 👌

jd-alexander · 2019-12-19T19:07:03Z

.../src/main/java/org/wordpress/android/ui/stats/refresh/utils/PostingActivityBlockAnnouncer.kt

+        setupViewDelegate()
+    }
+
+    private fun setupViewDelegate() {


Most of the logic that does the announcement lives within this function. As reviewers how do you feel about this? I have been thinking and I was wondering if it's worth breaking this up a bit more with some tests as well or since the functionality works and it achieves the goal of announcing each daily stats then it okay. Let me know :)

planarvoid

Thanks for the changes @jd-alexander, I've originally omitted this card for accessibility on purpose because I think its value is very questionable. If you use the app without talkback, the purpose of this card is to generally see how active your site was in the past months. It's impossible to see what the single day boxes mean (find what date they represent is very hard), wha the number of views on these days are, and you can't click them. It's purely a visual component and I still believe it should be omitted. If we really want to implement it, I'd approach it differently. First of all, I think reading all the items on click and saying the number of values is not useful and is a completely different use case. What about reading only the top days? Maybe ordered by views. Something like:
"The days with very high views for January are: the 1st, the 3rd...". On click you'd read "The days with high views for January are: the 5th, the 7th ...". This would reduce the number of clicks the user has to do and make the talk back semi-useful. The actual number of views can be reached through a different use case.
From the code point of view, I think we should build the list of content descriptions for a month in the use case (because building it is business logic). We should pass this list on the Block element and not have granular content descriptions on the Boxes (since they are really still not clickable). PostingActivityBlockAnnouncer doesn't have to be specific to posting activity any more, you'd just pass in a list of content descriptions and a view they are tied to and it would iterate through them on click. I also don't think it needs to be injected since it can't be tested anyway (and it would be used purely as view logic). I'd generally inject classes for 2 reasons, one is they can be mocked in unit tests and the second one is if their construction is too complex (and needs other injected classes). This one is neither.

Let me know if all of this makes sense!

planarvoid · 2019-12-19T12:35:23Z

WordPress/src/main/java/org/wordpress/android/ui/stats/refresh/lists/StatsBlockAdapter.kt

-class StatsBlockAdapter(val imageManager: ImageManager) : Adapter<BaseStatsViewHolder>() {
+class StatsBlockAdapter(
+    val imageManager: ImageManager,
+    private val postingActivityBlockAnnouncer: PostingActivityBlockAnnouncer


This class definitely shouldn't be added here since it's not related to all the item types. It's a very specific logic relevant to only Box Item so it should be passed in there.

Makes sense. Thanks 🙏

planarvoid · 2019-12-19T12:35:51Z

WordPress/src/main/java/org/wordpress/android/ui/stats/refresh/lists/StatsListFragment.kt

@@ -32,6 +33,7 @@ import javax.inject.Inject
 class StatsListFragment : DaggerFragment() {
    @Inject lateinit var viewModelFactory: ViewModelProvider.Factory
    @Inject lateinit var imageManager: ImageManager
+    @Inject lateinit var postingActivityBlockAnnouncer: PostingActivityBlockAnnouncer


Also remove this, related to the comment above.

planarvoid · 2019-12-19T12:36:20Z

...ress/src/main/java/org/wordpress/android/ui/stats/refresh/lists/sections/BlockListAdapter.kt

-class BlockListAdapter(val imageManager: ImageManager) : Adapter<BlockListItemViewHolder>() {
+class BlockListAdapter(
+    val imageManager: ImageManager,
+    private var postingActivityBlockAnnouncer: PostingActivityBlockAnnouncer


Also remove this, related to the comment above.

planarvoid · 2019-12-19T12:38:20Z

WordPress/src/main/java/org/wordpress/android/ui/stats/refresh/lists/sections/BlockListItem.kt

@@ -233,7 +233,8 @@ sealed class BlockListItem(val type: Type) {
    data class LoadingItem(val loadMore: () -> Unit, val isLoading: Boolean = false) : BlockListItem(LOADING_ITEM)

    data class ActivityItem(val blocks: List<Block>) : BlockListItem(ACTIVITY_ITEM) {
-        data class Block(val label: String, val boxes: List<Box>)
+        data class Block(val label: String, val boxItems: List<BoxItem>)
+        data class BoxItem(val box: Box, val contentDescription: String? = null)


Only the top level items have the Item name. I think this one could be still Box since it's basically a box (from the UI point of view). It's not a list item. I think having a Box and BoxType should be good enough.

planarvoid · 2019-12-19T12:43:36Z

...Press/src/main/java/org/wordpress/android/ui/stats/refresh/utils/ContentDescriptionHelper.kt

+    fun buildContentDescription(month: String, days: Int, posts: Int): String {
+        return if (posts > 1) {
+            resourceProvider.getString(
+                    R.string.stats_posting_activity_description_plural,


This doesn't belong here. This is specific to PostingActivity so it doesn't belong to the reusable Helper class. Either make the method more abstract and pass all the params or move it to the use case.

planarvoid · 2019-12-19T12:52:23Z

...java/org/wordpress/android/ui/stats/refresh/lists/sections/viewholders/ActivityViewHolder.kt

-class ActivityViewHolder(val parent: ViewGroup) : BlockListItemViewHolder(
+class ActivityViewHolder(
+    val parent: ViewGroup,
+    private var postingActivityBlockAnnouncer: PostingActivityBlockAnnouncer


Also remove this, related to the comment above.

jd-alexander · 2019-12-23T12:56:53Z

Thanks for the changes @jd-alexander, I've originally omitted this card for accessibility on purpose because I think its value is very questionable. If you use the app without talkback, the purpose of this card is to generally see how active your site was in the past months. It's impossible to see what the single day boxes mean (find what date they represent is very hard), wha the number of views on these days are, and you can't click them. It's purely a visual component and I still believe it should be omitted. If we really want to implement it, I'd approach it differently. First of all, I think reading all the items on click and saying the number of values is not useful and is a completely different use case. What about reading only the top days? Maybe ordered by views. Something like:
"The days with very high views for January are: the 1st, the 3rd...". On click you'd read "The days with high views for January are: the 5th, the 7th ...". This would reduce the number of clicks the user has to do and make the talk back semi-useful. The actual number of views can be reached through a different use case.

That's a good idea @planarvoid I originally decided to go with this implementation since it's approach was merged into the iOS app.

From the code point of view, I think we should build the list of content descriptions for a month in the use case (because building it is business logic). We should pass this list on the Block element and not have granular content descriptions on the Boxes (since they are really still not clickable).

Okay understood!

PostingActivityBlockAnnouncer doesn't have to be specific to posting activity any more, you'd just pass in a list of content descriptions and a view they are tied to and it would iterate through them on click. I also don't think it needs to be injected since it can't be tested anyway (and it would be used purely as view logic). I'd generally inject classes for 2 reasons, one is they can be mocked in unit tests and the second one is if their construction is too complex (and needs other injected classes). This one is neither.

Good point!

Let me know if all of this makes sense!

Yes, it does. Overall, what's the decision that's normally taken when there's a variation with an implementation on one platform? In this case, I could do the month content description approach but it would be definitely from that of iOS but it should be fine since it's accessibility right?

planarvoid · 2020-01-03T08:19:52Z

Overall, what's the decision that's normally taken when there's a variation with an implementation on one platform? In this case, I could do the month content description approach but it would be definitely from that of iOS but it should be fine since it's accessibility right?

@jd-alexander that's a very good question. I personally don't think it's super important to have everything 100% the same. I generally try to think about the changes (if they make sense) and if I think something doesn't look right, I give feedback to the designer (or to the iOS team). Sometimes they overlook a better solution and it's fine to iteratively improve one platform based on the findings of devs on the other. I think this is especially important in the accessibility improvements because even little changes to the usability can make the lives of the users so much more difficult/easy (controlling the app without seeing the screen is hard). In this case I'd suggest this solution to the iOS team (maybe you can also document it in a post) but I also think it's fine to have the solutions slightly different.

jd-alexander · 2020-01-07T14:15:06Z

@planarvoid thanks again for the review and thoughts. They have been very helpful. I implemented the solution you discussed in the comments above. I created another PR since this one was specific to the approach being taken. Let me know what you think 😄 I will be closing this one.

jd-alexander added 15 commits December 17, 2019 14:48

Created skeleton for the announcer that will traverse each block

faf7610

Added the announcer to the respective fragment, adapter and viewholder

65160ee

Connected blocks of activity viewholder to the announcer being created

eb5becd

Added block views to the announcer to listen to accessibility events

ff51d43

Refactored box model for to support content description

1bec852

renamed box variable to boxItem

4f35062

made the blocks accessible

b233175

marked block layout as important for accessibility but disabled parent

1e1b90a

Added the announcer dependency to the respective constructors

c653504

box now announces it's content description when tap cycles through them

cef0b3c

Removed unneeded when clause since only one event being observed

e30294c

Made content description nullable as invisible items wont need to set it

843ed0e

Added functionality in contentDescriptionHelper to generate announcement

adec26c

Added content description to the respective box

17e28a0

Separated date with comma

46dec8e

jd-alexander added [Type] Enhancement Accessibility Stats Refresh TalkBack Accessibility issues labels Dec 18, 2019

jd-alexander added this to the 14.0 milestone Dec 18, 2019

jd-alexander added 9 commits December 18, 2019 12:01

Added click action that describes what clicking actually does.

95858a8

Added announcement when the user has reached the end of the stats

c55759a

Utilized resource provider to get strings.

3c93f26

Added the ability to detect if there are no stats found and announce it.

5a3ea01

Utilized inc() instead of ++

b6adbfe

Fixed lint errors.

1f12cf3

Fixed failing tests due to changes with the contentDescriptionHelper

d92d024

Renamed boxes to boxItems

5e94761

Removed inc() since it doesn't mutate the value of the receiver

fd617e4

Removed unnecessary period.

d85dbe5

wordpress-mobile deleted a comment from wpmobilebot Dec 18, 2019

jd-alexander added 3 commits December 18, 2019 13:35

Fixed lint errors.

19dca31

Fixed parameter issue.

022db8b

Removed unused string.

bf13fa5

jd-alexander marked this pull request as ready for review December 18, 2019 19:14

jd-alexander requested a review from malinajirka December 18, 2019 19:16

jd-alexander commented Dec 19, 2019

View reviewed changes

malinajirka self-assigned this Dec 19, 2019

malinajirka requested a review from planarvoid December 19, 2019 10:15

jd-alexander commented Dec 19, 2019

View reviewed changes

malinajirka removed their assignment Dec 20, 2019

planarvoid requested changes Dec 20, 2019

View reviewed changes

malinajirka removed their request for review January 6, 2020 07:06

jd-alexander mentioned this pull request Jan 7, 2020

Accessibility : Posting Stats Improvement v2 #11049

Merged

2 tasks

jd-alexander closed this Jan 7, 2020

jd-alexander deleted the issue-10975/stats_post_accessibility branch January 24, 2020 02:54

irfano added Stats and removed Stats Refresh labels Mar 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Accessibility : Posting Activity Stats Improvement #10980

Accessibility : Posting Activity Stats Improvement #10980

Accessibility : Posting Activity Stats Improvement #10980

Accessibility : Posting Activity Stats Improvement #10980

Conversation

Findings

Solution

Testing

Reviewing

Submitter Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment