[SPIKE] Evaluate ability to have non-browser tabs in the tabs tray

[jonalmeida]

For https://github.com/mozilla-mobile/fenix/issues/14117, we want to have Synced Tabs be a separate tab option in the tabs tray.

With the current tabs tray, we fake switching to two different tabs tray using the TabsFeature.defaultTabsFilter to filter between the two types. Now that we want to switch between other non-browser tab data, we should do this better:

A list of things to consider if it's worth doing:

• Refactor the tabs tray to have separate views for each browser tab type.
• Use a ViewPager2 to switch between the multiple sources.
• Move multi-select to be re-useable for the above items. We could potentially use the RecyclerView selection library here as well.

Break these out into separate issues where needed.

┆Issue is synchronized with this Jira Task

[jonalmeida]

I spent some time evaluating the current state of the tabs tray and how we can start cleaning it up, so I wrote up a short doc of things that we can fix in smaller parts to get there.

The aim to break up the strong coupling of all the tabs tray UI into more manageable pieces, allow for unit testing, and aid easier perf analysis and fixes.

Wondering what other folks think about this since it won't be a small change and I'd like to discuss it before starting. Pinging @ekager and @gabrielluong for comment who are most familiar with the tabs tray right now but this is an open discussion for all. :)

---

1. Move more state to lib-state. See TabsTrayStore section below.
2. Pass interactor around to perform actions on the browserState and trayState where needed instead of view references.
3. Add middleware to perform "global" tabs tray actions without needing to pass around view references.
   • For example CloseTrayAction, CloseTrayAndGoHomeAction.
4. Use BrowserMenu2 for update the three-dot menu as the pager position changes.
   • The API is easier to use and update the menu and we need to move to the one eventually.
5. Use SecureWindowFeature for hiding tabs tray in private mode instead of custom implementation.
6. Convert multi-select to use RecyclerView multi-select library.
   • I played around with the library and it seems to do everything we want without needing a custom implementation.
   • Using the selection library might make our selections more usable if we want to use it for the other tray lists.
7. Rename browser TabsTray items to browser TabsList to distinguish between non-browser content like Synced Tabs list.
   • Current TabTrayView, TabTrayViewholder, et al. will be renamed to BrowserTabsList, BrowserTabsTrayViewholder, et al.
   • The TabsTray (note the plural) will be the higher level for all tray items.
8. Move CTA UI to be as part of each ViewPager child layout or as an adapter item for the same child.
9. Move Collections logic to a separate controller.

Technical Design

TabsTrayStore

• TabTrayDialogFragmentStore -> TabsTrayStore
• Remove BrowserState as a child state, and use it directly where needed.
• new/add collection result, for showing a snackbar.
• Multi-select state (already there)
• "hasLoaded" state - I don't think we need this if we fix accessibility.
• CTA state

Layout

A basic layout of how the views will be structured.

This should be straight-forward: a view-pager with individual layout containers in them (mostly RecyclerViews).

0 : Normal tab indicator P : Private tab indicator S : Synced Tabs indicator

+---+  +---+  +---+           + 
| 0 |  | P |  | S |           +  <---------+ BrowserMenu2
+---+  +---+  +---+           + 

+------------------------------+
|                              |
|                              |
|                              |
|                              |
|                              |
|                              |
|                              |
|                              |
|         ViewPager2           |
|                              |
|                              |
|                              |
|                              |
|                              |
|                              |
|                              |
|                              |
+---------------+--------------+
                ^
                |
+---------------+-------------------------------------------+
+                                                           +

+-----------------+  +-----------------+  +-----------------+
|                 |  |                 |  |                 |
|                 |  |                 |  |                 |
|                 |  |                 |  |                 |
|                 |  |                 |  |                 |
|                 |  |                 |  |                 |
|  Normal tabs    |  |  Private tabs   |  |  Synced tabs    |
|                 |  |                 |  |                 |
|(BrowserTabsList)|  |(BrowserTabsList)|  |(SyncedTabLayout)|
|                 |  |                 |  |                 |
|                 |  |                 |  |                 |
|                 |  |                 |  |                 |
|                 |  |                 |  |                 |
+-----------------+  +-----------------+  +-----------------+

TabsTrayContainer

• Interface for all tray items to implement so that we can take out implementations/adapters from the ViewPager2 adapter.
• Maybe we don't need this?

BaseBrowserTabsList

• Adapter: FenixTabsAdapter
• Abstract class, abstract TabsFeature filter.
• Basic tabs list that needs to have shared content between normal/private.
• FAB icon.
• RecyclerViewAccessibilityDelegate - for correctly selecting the selected tab. I don't think this is super necessary if we fix the notification when the initial tabs list is loaded (hasLoaded).

NormalBrowserTabsList

• Inherits from BaseBrowserTabsList.
• TabsFeature filter: { true }
• CTA for Recently Closed Tabs.
• Save to Collections button (note: see if we still need this duplicated button)
• Multi-select.

PrivateBrowserTabsList

• Inherits from BaseBrowserTabsList.
• TabsFeature filter: { it.content.private == true }
• SecureWindowFeature
• Other things?

SyncedTabsList

• Adapter: SyncedTabsAdapter
• Implements SyncedTabsView.

Accessibility

• TBD - we need to use an accessibilty delegate so that we can scroll through tabs easily. I think the only thing we need to consider is the selected tab, but we can come back to this later when our design is more concrete.

---

Notes:

• Making a RecyclerView's ViewHolder lifecycle-aware: https://code.luasoftware.com/tutorials/android/android-data-binding-for-recyclerview-with-livedata/
• Consider adding the preferences state to the tabs tray for the initialState (e.g. layout option, tab ordering).

[ekager]

I played around with the library and it seems to do everything we want without needing a custom implementation.

I'm glad that you were able to get the RecyclerView selection working! And you can trigger it externally via a button?

(note: see if we still need this duplicated button)

If we move towards these designs then we will need the "Select Tabs" in the top bar.

One remaining question for me with those designs would be if the entire bottom navigation disappears when in multiselect.

I'm happy to move forward with modularization and can help where needed!

[liuche]

So that I can get a clearer idea of order of operations here, we need this 1) non-browser tabs refactor before we can implement the 2) New Collections Figma mocks that emily is linking to, correct?

Vesta would like to get a prototype to test the new collections ideas (from the mocks) so we should figure out how we can get to that state. I assume it doesn't make sense to do the collections prototype outside of the refactor, right? (And even if it does, if we want to eventually want to release something similar to those mocks, we're better off doing 1 & 2 in order.)

[liuche]

Talked about this during planning, and since we don't want to have to redo a ton of work by prototyping without this in place, we'll do this in order (and this would also unlock synced tabs in tabs tray). @ekager will work with @jonalmeida to review this proposal and give feedback, as well as filing what the next steps are so we can start working on them asap.

[jonalmeida]

Sorry, I think I missed some notifications from yesterday until I received the last one now!

---

@ekager I noticed we have some CTA banner logic for recently opened tabs and thought about putting that in the state object as well, but thinking about it again, it seems like this is just a one-time check based on a pref and we can probably just dispatch an action to tell us to show the banner or not. Wondering if you have ideas about this or potential future work that we could plan for. 🙂

I'm glad that you were able to get the RecyclerView selection working! And you can trigger it externally via a button?

Yes, there's an API that let's you define the single-click property. You can see this in the Sunflower demo app which uses the selection library that we can modify with the patch below in the adapter. We would obviously do this dynamically depending on the state we have in the store.

@@ -93,6 +94,9 @@ class GardenPlantingAdapter :
             object : ItemDetailsLookup.ItemDetails<Long>() {
                 override fun getPosition() = adapterPosition
                 override fun getSelectionKey() = itemId
+                override fun inSelectionHotspot(e: MotionEvent): Boolean {
+                    return true
+                }
             }

I'm not super tied to the selection library, but it would be grand if we could make one implementation re-usable or use this opportunity to make our own solution re-usable if (but more likely when) we decide to have multi-selection for private tabs or synced tabs. We could just attach that modular bit and let those ViewHolders style themselves.

Regardless, I think we still need to store the selection in the tabs tray state so we can perform our own actions on it.

If we move towards these designs then we will need the "Select Tabs" in the top bar.

I think we can still accomplish that. My concern was that we wanted to stick to having the 'Add to collections' button at the bottom of the tabs and it causes a visual quirk that I'd rather avoid trying to build a hacky fix.

---

Talked about this during planning, and since we don't want to have to redo a ton of work by prototyping without this in place, we'll do this in order (and this would also unlock synced tabs in tabs tray). ekager will work with jonalmeida to review this proposal and give feedback, as well as filing what the next steps are so we can start working on them asap.

@liuche This sounds great! It would be good to think of the proposal above open to anyone on the team is interested as well that has concerns, from there Emily and I can limit our scope from there. 🙂 @boek worked on the tray originally so I wonder if there are any gotchas he may have come across then that we skipped here.

[jonalmeida]

I'm happy to move this forward if there are no other comments and there are no other considerations to make. Will file new breakdown issues and put this into the backlog to pick up next.

[liuche]

Thank you! If you can throw them into the Engineering Triage board > Skittle backlog I can add them to the eng backlog to pull in during planning.

[liuche]

@jonalmeida I'll be gathering items today for sprint planning tomorrow - can you file these today, and come to sprint planning to propose/explain them?

[jonalmeida]

I'm going to close this with all the issues I've filed so far. You can find the full list of issues on this board: https://github.com/mozilla-mobile/fenix/projects/38