Add "Refresh" accessibility menu #3121

nikclayton · 2022-12-29T12:43:53Z

Per https://developer.android.com/reference/androidx/swiperefreshlayout/widget/SwipeRefreshLayout the layout does not provide accessibility events, and a menu item should be provided as an alternative method for refreshing the content.

In fragments:

Implement the MenuProvider interface so it can populate the action bar menu in activities that host the fragment
Create a "Refresh" menu item, and refresh the state when it is selected

In activities:

Call setSupportActionBar if not already done, so the toolbar participates in menus
Set the title via supportActionBar if appropriate
Implement MenuProvider as well if the activity also provides menus

Per https://developer.android.com/reference/androidx/swiperefreshlayout/widget/SwipeRefreshLayout the layout does not provide accessibility events, and a menu item should be provided as an alternative method for refreshing the content. In `TimelineFragment`: - Implement the `MenuProvider` interface so it can populate the action bar menu in activities that host the fragment - Create a "Refresh" menu item, and refresh the state when it is selected `MainActivity` has to change how the menu is created, so that fragments can add items to it. In `MainActivity`: - Call `setSupportActionBar` so `mainToolbar` participates in menus - Implement the `MenuProvider` interface, and move menu creation there - Set the title via supportActionBar

nikclayton · 2022-12-29T13:07:49Z

This does the work for MainActivity/TimelineFragment.

Before I do the others, there is a RefreshableFragment interface, where some of this code could live. I looked in to this.

Options I looked at:

Create methods in the interface to be called from each fragment's onViewCreated, onCreateMenu, and onMenuItemSelected.

But the methods couldn't have the same names as the framework methods, and would need to take additional parameters (the fragment, the activity context, the ID of the SwipeRefreshLayout). This feels clunky.

Also, there's no way for the tooling to remind you to call these methods.

Convert RefreshableFragment to an abstract class, and implement some of the functionality in there.

Attractive, but doesn't work because these fragments already inherit from SFragment, and can only inherit from one class.

This menu code could be moved in to SFragment, and SFragment could expose a "hasRefreshMenu" property that subclasses could set to true and would activate the functionality in SFragment.

But my understanding is that SFragment is due a refactoring, so things that complicate the SFragment code and its relationship with subclasses is probably not a good idea.

Duplicate the code in the fragments and activities that need it.

On the one hand, I'm not a huge fan of duplicating code. On the other hand, it's probably the cleanest solution at the moment, and it does mean that if you're reading the code its very clear where the refresh menu is coming from without having to chase down layers of nesting.

Thoughts?

nikclayton · 2022-12-30T08:32:04Z

Option 3 it is. Done.

# Conflicts: # app/src/main/java/com/keylesspalace/tusky/MainActivity.kt # app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineFragment.kt # app/src/main/res/values/strings.xml

Per guidelines in https://developer.android.com/develop/ui/views/touch-and-input/swipe/add-swipe-interface#AddRefreshAction

nikclayton · 2023-02-02T11:15:15Z

Also, fix the colour of the refresh progress indicator

Make `tabs` `var` instead of `val` in `MainPagerAdapter` so it can be updated when tabs change. Then detach the `tabLayoutMediator`, update the tabs, and call `notifyItemRangeChanged` in `setupTabs()`. This fixes a bug (not sure if it's this code, or in ViewPager2) where assigning a new adapter to the view pager seemed to result in a leak of one or more fragments. This wasn't user-visible, but it's a leak, and it becomes user-visible when fragments want to display menus. This also fixes two other bugs: 1. Be on the left-most tab. Scroll down a bit. Then modify the tabs at "Account preferences > tabs", but keep the left-most tab as-is. Then go back to MainActivity. Your reading position in the left-most tab has been jumped to the top. 2. Be on any non-left-most tab. Then modify the tab list by reordering tabs (adding/removing tabs is also OK). Then go back to MainActivity. Your tab selection has been overridden, and the left-most tab has been selected. Because the fragments are not destroyed unnecessarily your reading position is retained. And it remembers the tab you had selected, and as long as that tab is still present you will be returned to it, even if it's changed position in the list. Fixes #3251

Using "collapseActionView" requires the user to press "Back" twice to exit the activity, which is not acceptable.

Previous code had the toolbar in the fragment's layout. Refactor to make consistent with other activities, and move the toolbar in to the activity layout. Implement MenuProvider in ViewThreadFragment to adjust the menu in the activity.

# Conflicts: # app/src/main/java/com/keylesspalace/tusky/MainActivity.kt # app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineFragment.kt

Be consistent with the layout resource files, which have an activity/fragment prefix, then the lower_snake_case name of the activity or fragment it's for.

nikclayton · 2023-02-05T14:11:47Z

Should be squash merged

connyduck · 2023-02-10T20:39:48Z

I'm still not convinced this is even a problem 🤔
And if it is, why can't it be solved by accessibility actions?

nikclayton · 2023-02-11T11:27:04Z

I'm still not convinced this is even a problem 🤔 And if it is, why can't it be solved by accessibility actions?

https://developer.android.com/reference/androidx/swiperefreshlayout/widget/SwipeRefreshLayout has the explanation:

"The SwipeRefreshLayout does not provide accessibility events; instead, a menu item must be provided to allow refresh of the content wherever this gesture is used."

connyduck · 2023-02-14T20:48:13Z

Yes I have seen that, I don't think it is a good idea (and I don't know a single app that does that). I'd leave it as it is, or add an accessibility action.

nikclayton · 2023-02-15T09:17:04Z

Yes I have seen that, I don't think it is a good idea (and I don't know a single app that does that).

Web browsers. I just checked Chrome and Firefox on my device, they both have "Refresh" or equivalent functionality accessible by swipe and menu access.

add an accessibility action.

You keep saying this, but I don't understand how that would work in practice.

My understanding is that accessibility actions need to be added to a view. But there's no available view to add a "swipe to refresh" accessibility action to.

It doesn't make sense on any of the individual list items
Pressing a tab is already used to go to the top of the list, and is not discoverable
Long-pressing a tab is already used to show the tab description ("Home", "", etc), and is not discoverable
Pressing the avatar opens the side drawer
Making the header clickable would be extremely non-standard, and too easy to do by accident

All those problems are why the guidelines say "Add a menu item".

So I feel like I'm missing something. When you say "add an accessibility action", which view, specifically, are you suggesting that the accessibility action should be added to?

# Conflicts: # app/src/main/java/com/keylesspalace/tusky/MainActivity.kt

connyduck · 2023-02-20T18:19:04Z

I tried putting it on the main toolbar, Talkback goes "Home, press to activate, actions available".

Making the header clickable would be extremely non-standard, and too easy to do by accident

We jump to top when clicking it in MainActivity. I see no problem with that and it would make sense to do the same in other views as well.

nikclayton · 2023-02-20T18:37:27Z

But it's not just about Talkback:

You should add the refresh action as a menu item, rather than as a button, by setting the attribute android:showAsAction=never. If you display the action as a button, users may assume that the refresh button action is different from the swipe-to-refresh action. By making the refresh action less conspicuous in the action bar, you can encourage users to perform manual updates with the swipe gesture while still maintaining the accessible option in a place where D-pad users would look for it. - https://developer.android.com/develop/ui/views/touch-and-input/swipe/add-swipe-interface#AddRefreshAction

(emphasis mine)

The whole point of the menu is to provide an accessible and discoverable alternative to swiping. Any solution that is not both accessible and discoverable doesn't make sense.

What, specifically, is the objection to the menu on these fragments? Other fragments in the app have menus.

connyduck · 2023-02-21T19:01:34Z

I think it is confusing to have options in the same menu where some apply to the current fragment and others have nothing to do with (in MainActivity), also I would not expect to have a menu item for things I can also do otherwise. But I guess I'll have to live with it because accessibility is more important.

nikclayton · 2023-02-21T22:14:53Z

I think it is confusing to have options in the same menu where some apply to the current fragment and others have nothing to do with (in MainActivity),

That doesn't happen.

This code uses the MenuProvider / MenuHost interfaces, and because those are tied to the lifecycle of the fragment, if the fragment is paused any menu items added by that fragment are automatically removed.

That's this code in the fragments:

requireActivity().addMenuProvider(this, viewLifecycleOwner, Lifecycle.State.RESUMED)

and the 3-arg version of addMenuProvider() is described in https://developer.android.com/reference/androidx/core/view/MenuHost#addMenuProvider(androidx.core.view.MenuProvider,androidx.lifecycle.LifecycleOwner,androidx.lifecycle.Lifecycle.State).

nikclayton · 2023-02-21T22:18:51Z

Just in case you want to convince yourself of this, fetch this branch, modify NotificationsFragment.java to remove the addMenuProvider call, and then build/launch.

Then swipe between the fragments in MainActivity. You'll see that all the tabs, except the notification tab have the menu, and the menu disappears when you're on the notifications tab, and reappears when you're on one of the others.

connyduck · 2023-02-23T17:57:54Z

Yes I know it is technically a different menu, still shows up in the same place

connyduck · 2023-02-23T18:08:37Z

app/src/main/java/com/keylesspalace/tusky/components/account/media/AccountMediaFragment.kt

@@ -133,6 +148,27 @@ class AccountMediaFragment :
        }
    }

+    override fun onCreateMenu(menu: Menu, menuInflater: MenuInflater) {


AccountMediaFragment and other fragments in the AccountActivity should only be refreshed together. If we are doing this we need to at least be consistent

MainActivity: Every Fragment has an individual swipe to refresh. The new menu refreshes the individual, active fragment ✅
AccountActivity: There is one swipe to refresh. It refreshes the activity as well as all the fragments. The menu options just refreshes the active fragment now 🟥 If it is supposed to improve the accessibilty of the swipe to refresh, it needs to refresh the same thing.

Actually when I think about it, having the menu option refresh all fragments, even in MainActivity and SearchActivity, could be a good idea and would lessen my concerns of it being confusing.

AccountActivity: There is one swipe to refresh. It refreshes the activity as well as all the fragments. The menu options just refreshes the active fragment now 🟥 If it is supposed to improve the accessibilty of the swipe to refresh, it needs to refresh the same thing.

That makes sense. It's an oversight that it doesn't. I'll fix that. [Edit: done]

Actually when I think about it, having the menu option refresh all fragments, even in MainActivity and SearchActivity, could be a good idea and would lessen my concerns of it being confusing.

That contradicts the previous paragraph, where you just said "it needs to refresh the same thing".

The menu should behave identically to the swipe gesture.

connyduck · 2023-02-23T18:14:24Z

app/src/main/res/menu/fragment_account_media.xml

+        android:id="@+id/action_refresh"
+        android:title="@string/action_refresh"
+        app:showAsAction="never" />
+</menu>


Can we have 1 refresh menu instead all these duplicate files please?

I thought about that, but I figured:

I plan on extending some of these to add additional menu items:

For example:

in Notifications the "Clear" and "Filter" controls can move in to the menu

on a timeline having a menu with "Hide boosts" and "Hide replies" (or "Show...") to temporarily override (or adjust) the preference

when viewing edits, an option to toggle the display of what's changed

etc.

It makes it easier to navigate the code. If you want to see what menu an activity or fragment uses you only have to open one file.

Some timelines don't have swipetorefresh enabled (e.g., those shown on AccountActivity). In those cases don't add the refresh menu, rely on the hosting activity to provide it. Update AccountActivity to provide the refresh menu item.

Nik Clayton added 2 commits February 2, 2023 12:01

Merge branch 'develop' into a11y-swipe-refresh

2c93504

# Conflicts: # app/src/main/java/com/keylesspalace/tusky/MainActivity.kt # app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineFragment.kt # app/src/main/res/values/strings.xml

Never show the refresh item as a menubar action

99a7eca

Per guidelines in https://developer.android.com/develop/ui/views/touch-and-input/swipe/add-swipe-interface#AddRefreshAction

Nik Clayton added 18 commits February 2, 2023 12:59

Add "Refresh" menu item for AccountMediaFragment

9fe1e76

Also, fix the colour of the refresh progress indicator

Implement "Refresh" for AnnouncementsActivity

b28de70

Add "Refresh" menu for ConversationsFragment

e8af34e

Merge branch '3251-leaking-fragments' into a11y-swipe-refresh

aace6b0

Add "Refresh" menu for ScheduledStatusActivity

b8d73d4

Lint

2d83786

Add "Refresh" menu for SearchFragment / SearchActivity

6cde1f0

Explicitly set the searchview width

70e71f5

Using "collapseActionView" requires the user to press "Back" twice to exit the activity, which is not acceptable.

Merge branch 'develop' into a11y-swipe-refresh

11a510d

Move toolbar handling in to ViewThreadActivity

03be783

Previous code had the toolbar in the fragment's layout. Refactor to make consistent with other activities, and move the toolbar in to the activity layout. Implement MenuProvider in ViewThreadFragment to adjust the menu in the activity.

Add "Refresh" menu to ViewThreadFragment

2ba4226

Implement "Refresh" for ViewEditsFragment

65da134

Lint

73b2944

Merge branch 'develop' into a11y-swipe-refresh

14ca3f3

# Conflicts: # app/src/main/java/com/keylesspalace/tusky/MainActivity.kt # app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineFragment.kt

Add "Refresh" menu to ReportStatusesFragment

cd43957

Add "Refresh" menu to NotificationsFragment

8b91ecb

Rename menu resource files

716f010

Be consistent with the layout resource files, which have an activity/fragment prefix, then the lower_snake_case name of the activity or fragment it's for.

nikclayton changed the title ~~Add "Refresh" accessibility menu to TimelineFragment~~ Add "Refresh" accessibility menu Feb 5, 2023

nikclayton marked this pull request as ready for review February 5, 2023 14:11

Merge branch 'develop' into a11y-swipe-refresh

a7f5fe9

# Conflicts: # app/src/main/java/com/keylesspalace/tusky/MainActivity.kt

connyduck reviewed Feb 23, 2023

View reviewed changes

Nik Clayton added 2 commits February 27, 2023 21:00

Merge branch 'develop' into a11y-swipe-refresh

a0ff3a7

Only enable refresh menu if swiptorefresh is enabled

ba3318b

Some timelines don't have swipetorefresh enabled (e.g., those shown on AccountActivity). In those cases don't add the refresh menu, rely on the hosting activity to provide it. Update AccountActivity to provide the refresh menu item.

connyduck merged commit 1b6108c into tuskyapp:develop Mar 1, 2023

connyduck mentioned this pull request Mar 1, 2023

Cannot refresh timelines/notifications with TalkBack #1711

Closed

1 task

nikclayton deleted the a11y-swipe-refresh branch June 20, 2023 23:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "Refresh" accessibility menu #3121

Add "Refresh" accessibility menu #3121

nikclayton commented Dec 29, 2022 •

edited

Loading

nikclayton commented Dec 29, 2022

nikclayton commented Dec 30, 2022 •

edited

Loading

nikclayton commented Feb 2, 2023 •

edited

Loading

nikclayton commented Feb 5, 2023

connyduck commented Feb 10, 2023

nikclayton commented Feb 11, 2023

connyduck commented Feb 14, 2023

nikclayton commented Feb 15, 2023

connyduck commented Feb 20, 2023

nikclayton commented Feb 20, 2023

connyduck commented Feb 21, 2023

nikclayton commented Feb 21, 2023 •

edited

Loading

nikclayton commented Feb 21, 2023

connyduck commented Feb 23, 2023

connyduck Feb 23, 2023

nikclayton Feb 27, 2023 •

edited

Loading

connyduck Feb 23, 2023

nikclayton Mar 1, 2023 •

edited

Loading

Add "Refresh" accessibility menu #3121

Add "Refresh" accessibility menu #3121

Conversation

nikclayton commented Dec 29, 2022 • edited Loading

nikclayton commented Dec 29, 2022

nikclayton commented Dec 30, 2022 • edited Loading

nikclayton commented Feb 2, 2023 • edited Loading

nikclayton commented Feb 5, 2023

connyduck commented Feb 10, 2023

nikclayton commented Feb 11, 2023

connyduck commented Feb 14, 2023

nikclayton commented Feb 15, 2023

connyduck commented Feb 20, 2023

nikclayton commented Feb 20, 2023

connyduck commented Feb 21, 2023

nikclayton commented Feb 21, 2023 • edited Loading

nikclayton commented Feb 21, 2023

connyduck commented Feb 23, 2023

connyduck Feb 23, 2023

Choose a reason for hiding this comment

nikclayton Feb 27, 2023 • edited Loading

Choose a reason for hiding this comment

connyduck Feb 23, 2023

Choose a reason for hiding this comment

nikclayton Mar 1, 2023 • edited Loading

Choose a reason for hiding this comment

nikclayton commented Dec 29, 2022 •

edited

Loading

nikclayton commented Dec 30, 2022 •

edited

Loading

nikclayton commented Feb 2, 2023 •

edited

Loading

nikclayton commented Feb 21, 2023 •

edited

Loading

nikclayton Feb 27, 2023 •

edited

Loading

nikclayton Mar 1, 2023 •

edited

Loading