1810775 - (tabs-tray-to-compose) [Meta] Composify the tabs tray

Status: NEW

(Fenix :: Tabs, enhancement)

Description

[Jonathan Almeida [:jonalmeida]]

From github: https://github.com/mozilla-mobile/fenix/issues/21318.

To avoid breaking everything at once, we can incrementally introduce Compose into our tabs tray by following this order of converting the code:

1. [x] https://github.com/mozilla-mobile/android-components/issues/11012
   • Update TabsFeature to pass in TabSessionState instead of tabstray's Tab.
2. [x] https://github.com/mozilla-mobile/fenix/issues/21894
   • Individual tab viewholders: GridViewHolder, ListViewHolder.
3. [ ] https://github.com/mozilla-mobile/fenix/issues/21896
   • Gesture support - how to support swiping to delete by horizontal and vertical directions, see TabsTouchHelper
4. [x] https://github.com/mozilla-mobile/fenix/issues/21897
   • Inactive tab list - see InactiveTabsAdapter and it's viewholders.
5. [x] https://github.com/mozilla-mobile/fenix/issues/21898
   • Group lists - this is where gesture support can help us too with vertical swiping.
6. [ ] https://github.com/mozilla-mobile/fenix/issues/21899
   • Browser lists - see NormalBrowserTrayList, PrivateBrowserTrayList, AbstractBrowserTrayList.
7. [x] https://github.com/mozilla-mobile/fenix/issues/21900
   • Synced Tabs list - SyncedTabsTrayLayout, SyncedTabsAdapter
   • We can do this async; doesn't have to wait for the normal/private tab list.
8. [ ] https://github.com/mozilla-mobile/fenix/issues/21901
   • Normal/Private page list - NormalBrowserPageViewHolder, PrivateBrowserPageViewHolder, SyncedTabsPageViewHolder
9. [x] https://github.com/mozilla-mobile/fenix/issues/25891
10. [ ] https://github.com/mozilla-mobile/fenix/issues/26131
11. [ ] ⚠️ Profile tabs tray performance before/after a full Compose re-write OR during each incremental step.
    • Reading reference: https://engineering.premise.com/measuring-render-performance-with-jetpack-compose-c0bf5814933
    • There is an interesting analysis on perf for Compose in Pocket here: https://github.com/mozilla-mobile/fenix/pull/22067
12. [x] https://github.com/mozilla-mobile/fenix/issues/26087
13. [ ] https://github.com/mozilla-mobile/fenix/issues/26476

(file individual tickets and link them here)

┆Issue is synchronized with this Jira Task

Change performed by the Move to Bugzilla add-on.

Comment 1

[Noah Bond [:007]]

Here is my proposal for handling the rest of this rewrite:

1. Set up the initial forking of Compose vs XML tabs tray use path in the code, utilizing FeatureFlags.composeTabsTray inside of TabsTrayFragment
2. Create root bottom sheet style container UI that:
   • Expands with tapping the tabs tray button to the full height (https://bugzilla.mozilla.org/show_bug.cgi?id=1813753)
   • Collapses with swipes (possibly one swipe pending ^ ticket)
3. Set up Compose-based paging and have three pages working for normal/private/sync.
   • Have placeholder layouts for Normal/private/synced
   • Should we make this pager specific to tabs tray or a generic paging implementation? Tabs tray for now then generic as follow-up?
4. Plug-in existing SyncedTabs composable into tabs tray pager (can be done async after step 2)
5. Plug in existing InactiveTabs composable into tabs tray pager (can be done async after step 2)
6. Create normal browsing list Composable and plug into tabs tray pager (can be done async after step 2)
7. Create private browsing list Composable and plug into tabs tray pager (can be done async after step 2)
   7.5 Sync with product/UX to see what more needs to be done for this before it can land in Nightly
8. Add support for tab gestures
9. Add support for Tab reordering
10. Convert the FAB to Compose (can be done async)
11. Performance benchmarking?
12. Enable in Nightly
13. Enable in Beta
14. Release
15. Remove XML implementation from the code and any necessary follow-up clean up.

How does this look?

Comment 2

[Petru-Mugurel Lingurar [:petru]]

Great plan!
I think it would also help to know where we're at and what's next and this could be done through creating separate tickets and making this depending on those which could be closed one by one.

Comment 3

[Noah Bond [:007]]

Tabs Tray to Compose

Points of contact

Engineering - :007, :aputanu, :vdreghici, :avirvara
Reviewers - :matt-tighe, :gl
UX - Verdi
Product - Channing Bell and Stefan Smagula
QA - Mirabela Lobontiu and Delia Pop
Test Eng - Oana + AaronMT

Motivation

The Tabs Tray, along with the homescreen, the awesomebar, and the browser, is one of the core pillars of our app. It is a cornerstone of user functionality and a common area for feature enhancements. In its current state, the Tabs Tray’s XML-based UI code makes it difficult to iterate quickly, troubleshoot bugs, and add meaningful enhancements without bloating the codebase. Converting the Tabs Tray to 100% Compose would mitigate or completely eliminate many pain points as well as provide quality of life improvements. Specifically, the conversion would improve iteration speed, create a stateful and modular UI, and help simplify business logic.

Iteration speed is always an essential benchmark when considering refactors. A common question is “Will this rewrite simply be a technical project, or will it yield tangible dividends to engineering productivity?” By using Compose in our code, it means we have the full power of Kotlin to develop our UI, and we don’t have to context switch to another paradigm (e.g. XML) in order to create dynamic screens. Gone are the days of needing to spin up multiple files of boilerplate when writing UI in XML and needing to understand the arcana of View styling. The previewing tooling also allows us to quickly iterate on the UI and readily compare the current state of development against figmas and send screenshots to the design/UX team. We leveraged this with the “composified” Inactive Tabs, where we have a preview which allows us to test the behavior of the inactive tabs section as well as the embedded auto-close dialog. After 100% conversion, we’ll be able to have any number of previews that can be constructed for any possible combination of screen size, device orientation, theming, or language.

In a world where app data is updating constantly, it’s essential to have a UI that responds to state changes in real-time and only updates the pertinent sections. With Compose, we’ll be able to further leverage our Store and State pattern, enabling each component of the Tabs Tray to update only when it needs to. We’ve actually been able to do this already in this hybrid state of XML + Compose, utilizing our homebrewed ComposeViewHolder and observeAsComposableState. Consuming the state with the power of Kotlin optionals has already allowed us to create dynamic content on the Homescreen.

In tandem to the Tabs Tray being more stateful, it will also be significantly modular. Each distinct component will be constructed in such a way that allows for both granular and holistic previewing. This means that we’ll be able to locally validate the functionality and UI of each part of the Tabs Tray in isolation but also the tray in its entirety. The goal of the refactor will also be to reduce or completely eliminate UI code duplication. Using Compose + Kotlin, we’ll be able to abstract any number of visual components to functions or local variables to greatly increase code readability and sustainability while helping to prevent future regressions.

As part of making the Tabs Tray 100% Compose and therefore stateful, we’ll also be able to simplify the business logic. After the refactor, it will be fairly trivial to insert metrics, add experiments, and test feature enhancements isolated to Nightly. Our existing interactor/controller pattern will also be easier to refactor as there will no longer be the tight coupling to a specific, XML platform. Ideally, our state and interactor code should be UI framework agnostic, and this refactor will make it easier to accomplish this.

In the end, a Composified Tabs Tray provides the team an opportunity to develop a model for the future of not just Fenix’s UI but also Focus'. Compose provides a ton of quality of life improvements and will make adding enhancements to the Tabs Tray a considerably less painful process. We have already converted a couple parts of the Tabs Tray, and while we have a decent amount of work ahead, it will all be so this core pillar of our app can continue to flourish for the years to come.

Implementation plan

I've outlined below what work has already been completed and what work remains. My recommendation would be to get this out in Nightly as soon as possible to let it bake for a month or two before elevating it to Beta. I've indicated a phase when this should be turned on in Nightly at the absolute latest. There is a column in the Tickets remaining tables titled Sequential or parallel, used to indicate which tickets can be worked on simultaneously after a certain "milestone" ticket is completed. While this effort could be completed by a single engineer, having at least the occasional assist with some of the parallel work would help get the Tabs Tray rewrite into Nightly sooner. There is also a ticket to formally track when to sync with product and UX after the rewrite is ~80% completed. We will also want to conduct a formal UX review around 90-95% completion, or as soon as they're willing to preview the changes.

Success criteria

WIP spreadsheet to track the criteria highlighted below

The following criteria will determine if this rewrite is successful

Codebase footprint

Compared to the XML implementation, the composified Tabs Tray will have a fraction of the footprint it has today. Because the UI code will be 100% Kotlin and will not require a ton of boilerplate in order to function, the size in the codebase should, at most, be a half as large. Between the number of files and lines of code, we should see a substantial reduction of the amount we have to maintain for this feature.

Performance

Although Compose hasn't been around as long as XML to have the same amount of optimization, we should still see relatively the same performance after the rewrite, if not marginally better in some areas.

@Preview coverage

After the rewrite, the Tabs Tray will have 100% coverage of Compose previews. The Compose previews allow for quicker iteration of the UI's development and all of its future enhancements. By having the Tabs Tray and all of its subcomponents previewable, we'll be able to more easily find bugs but also empower UX with having more access to how the Tabs Tray and all of its use case permutations look like on Android.

Bugs in the Tabs Bugzilla component

After the rewrite is in Release, we should also see fewer bugs filed in Bugzilla. Since the codebase footprint will be smaller, it will inherently have fewer points of failure and require less long term maintenance. The UI will also be easier to demo thanks to previews, and we'll be catching visual bugs earlier in the development process as a result.

Accessibility Scanner infractions

As part of the Compose framework, accessibility is at the forefront between under-the-hood optimizations and IDE tooling. After this rewrite, by nature of the framework, there should be substantially fewer Accessibility Scanner infractions in the Tabs Tray.

Less quantifiable criteria

Reduced time to investigate bugs

As mentioned in the above section covering previews, the rewrite will make it easier to triangulate the source of bugs. The previews will allow us to identify exactly what level of abstraction the bug is occurring inside the Tabs Tray and quickly iterate on fixes. Additionally, since the UI code will be 100% Kotlin, we'll have access to all of the traditional debugging tools within Android Studio, allowing us to step through, line-by-line, what might be the source of a potential bug.

Easier Compose refactors in the future

By being the flagship Compose rewrite, the Tabs Tray will serve as an example for best practices going forward. Additionally, it allowed us to construct more top-level components from the Design team, which will expedite all future Compose work. The rewrite also highlighted pain points of our business logic architecture, and it will now be easier to flesh-out more of the Redux re-architecting and that work will be more informed after this feature is 100% Compose.

Feature Flag

composeTabsTray

Secret Setting

settings.enableTabsTrayToCompose

Secret Settings -> Enable Tabs Tray to Compose

Tickets already completed

Tabs Tray component	Composable	Ticket
Synced tabs	SyncedTabs	https://github.com/mozilla-mobile/fenix/issues/21900
Inactive tabs	InactiveTabs	https://github.com/mozilla-mobile/fenix/issues/21897
Tabs as list items	TabListItem	https://github.com/mozilla-mobile/fenix/issues/21898
Tabs as grid items	TabGridItem	https://github.com/mozilla-mobile/fenix/issues/25891

Tickets remaining

Note: these tickets are sequenced in a rough order of operations. Some tickets can be completed in parallel and are appropriately notated.

Phase 1 - Initial setup

Task	Ticket	Sequential or parallel	Dependency
Refactor the feature flag to be backed by a secret setting	Bug 1815968	Sequential	-
Set up the initial forking of Compose vs XML tabs tray use path in the code	Bug 1814986	Sequential	Bug 1815968
Create root, bottom-sheet-style container UI	Bug 1814987	Sequential	Bug 1814986

Phase 2 - Paging & UI Container setup

Task	Ticket	Sequential or parallel	Dependency
Set up Compose-based paging and have three pages working for normal/private/sync.	Bug 1814988	Parallel	Bug 1814987
Add tabs tray banner container for the page indicator and multi select UI	Bug 1816743	Parallel	Bug 1814987
Open TabsTray composable at the correct page when opened from the home page	Bug 1819194	Sequential	Bug 1814988
Open TabsTray composable at the correct page when opened from the browser	Bug 1826882	Sequential	Bug 1814988
Dismiss the tray when tapped outside	Bug 1819439	Parallel	Bug 1814987
Disable Theme.Private from being used in the Tabs Tray	Bug 1820897	Parallel	-

Phase 3 - Implement the bulk of the feature

Tab lists

Task	Ticket	Sequential or parallel	Dependency
Plug-in existing SyncedTabs composable into tabs tray pager	Bug 1814992	Parallel	Bug 1814988
Create normal browsing list Composable and plug into tabs tray pager	Bug 1814994	Parallel	Bug 1814988
Plug in existing InactiveTabs composable into tabs tray pager	Bug 1814993	Parallel	Bug 1814988, Bug 1814994
Create private browsing list Composable and plug into tabs tray pager	Bug 1814995	Sequential	Bug 1814994
Long titles in TabListItem and TabGridItem makes the app crash	Bug 1815620	Parallel	Bug 1814994
Implement tab list/grid click listeners	Bug 1821206	Sequential	Bug 1814994, Bug 1822878
Refactor SelectionHolder out of TabsTrayController	Bug 1822878	Parallel	-
Implement selection and multi-selection logic in TabList and TabGrid	Bug 1821425	Parallel	Bug 1814994, Bug 1821206
Scroll to the currently active tab	Bug 1822193	Parallel	Bug 1814994, Bug 1814993, Bug 1814995, Bug 1819194
Fix crash when rotating the Tabs Tray when the tabs are in a grid	Bug 1826457	Parallel	-
Add empty state UI for tab pages	Bug 1822192	Parallel	Bug 1814992, Bug 1814994, Bug 1814995
Implement tab deletion UNDO Snackbar	Bug 1823999	Parallel	Bug 1821206, Bug 1819194

Tabs Tray FAB

Task	Ticket	Sequential or parallel	Dependency
Create top level FAB Composable	Bug 1815011	Parallel	-
Add FAB to the Tabs Tray	Bug 1815013	Parallel	Bug 1814987, Bug 1815011
Port over the the FAB's business logic	Bug 1819196	Sequential	Bug 1815013

Tabs Tray banners

Task	Ticket	Sequential or parallel	Dependency
Add page indicator banner UI	Bug 1814991	Sequential	Bug 1816743
Add multi selection banner UI	Bug 1816522	Sequential	Bug 1816743
Create top level DropdownMenu Composable	Bug 1816567	Parallel	-
Refactor unnecessary selected tabs parameters out of SelectionMenuIntegration.handleMenuClicked and SelectionBannerBinding.initListeners	Bug 1822887	Parallel	-
Add functionality to the three-dot button in the tabs tray page indicator	Bug 1816517	Parallel	Bug 1814991, Bug 1816567
Add functionality for the close button in the multi selection banner	Bug 1826994	Parallel	Bug 1816522, Bug 1822887
Add functionality for the collections button in the multi selection banner	Bug 1816555	Parallel	Bug 1816522, Bug 1822887
Add functionality for the share button in the multi selection banner	Bug 1816556	Parallel	Bug 1816522, Bug 1822887
Add functionality for the three dot button in the multi selection banner	Bug 1816558	Sequential	Bug 1816522, Bug 1816567, Bug 1822887

Phase 4 - Checkpoint

Task	Ticket	Sequential or parallel	Dependency
Sync with product & UX to see what more needs to be done for this before it can land in Nightly	Bug 1814996	Sequential	Bug 1814994, Bug 1814991, Bug 1816522, Bug 1819196

Phase 5 - Post checkpoint / work needed to enable in Nightly

Feature work

Task	Ticket	Sequential or parallel	Dependency
Fix dropdown menu corner radius	Bug 1828698	Parallel	-
Hide the close button on tab grid items when in multi select mode	Bug 1826670	Parallel	-
Fix UI discrepancies of tab items	Bug 1828493	Parallel	Bug 1821425
Investigate janky Tabs Tray in Compose performance	Bug 1815579	Parallel	Bug 1814994
Add support for Tab reordering	Bug 1810780	Parallel	Bug 1814994

UI tests

Task	Ticket	Sequential or parallel	Dependency
Investigate current state of UI tests	Bug 1831602	Sequential	-
Convert ComposeTabbedBrowsingTest.closeAllTabsTest	Bug 1832606	Parallel	Bug 1831602
Convert ComposeTabbedBrowsingTest.verifyUndoSnackBarTest	Bug 1832608	Sequential	Bug 1831602
Convert ComposeTabbedBrowsingTest.verifyPrivateTabUndoSnackBarTest	Bug 1832610	Sequential	Bug 1831602
Convert ComposeTabbedBrowsingTest.closePrivateTabsNotificationTest	Bug 1832611	Sequential	Bug 1831602
Convert ComposeTabbedBrowsingTest.verifyTabTrayNotShowingStateHalfExpanded	Bug 1832612	Sequential	Bug 1831602
Convert ComposeTabbedBrowsingTest.verifyEmptyTabTray	Bug 1832613	Sequential	Bug 1831602
Convert ComposeTabbedBrowsingTest.verifyOpenTabDetails	Bug 1832615	Sequential	Bug 1831602
Convert ComposeTabbedBrowsingTest.verifyContextMenuShortcuts	Bug 1832616	Sequential	Bug 1831602
Convert ComposeTabbedBrowsingTest.closeTabTest	Bug 1832617	Sequential	Bug 1831602, Bug 1810776
Convert ComposeTabbedBrowsingTest.closePrivateTabTest	Bug 1832609	Sequential	Bug 1831602, Bug 1810776
Refactor consumers of TabDrawerRobot to use ComposeTabDrawerRobot instead	Bug 1832618	Parallel	Bug 1832606, Bug 1832617, Bug 1832609

Major experience bugs found during pre-Nightly QA

Task	Ticket	Sequential or parallel	Dependency
Incorrect "RESYNC" button is displayed in the synced tabs view	Bug 1837425	Parallel	-
Tabs tray is incorrectly displayed when changing the orientation to Landscape mode	Bug 1837437	Parallel	-

Sign offs

• [x] Product sign off
• [x] UX sign off
• [x] Formal QA request - Bug 1837237

Phase 6 - Bake in Nightly

Task	Ticket	Sequential or parallel	Dependency
Performance benchmarking	Bug 1814998	Sequential	Bug 1814995, Bug 1815579
Enable in Nightly	Bug 1815001	Sequential	Bug 1814996
Reveal secret setting in Beta and Release	Bug 1844667	Parallel	-

Phase 6.5 - Nightly bake tickets

All bugs found during the Nightly bake are tracked in Bug 1841141

High priority tickets / necessary for Beta

Task	Ticket	Sequential or parallel	Dependency
Double check tab list item specs	Bug 1826404	Parallel	-
Add support for tab gestures	Bug 1810776	Parallel	Bug 1814994
Add Tabs Tray tab auto Close banner	Bug 1815765	Parallel	Bug 1814994
Add Inactive Tabs CFR	Bug 1816746	Parallel	Bug 1814993
Fix tab grid item's close button size	Bug 1836351	Parallel	-
Re-align tab number text inside of the normal tabs page button	Bug 1836352	Parallel	-
Confirm text style of "X tabs selected" text in multi selection banner	Bug 1828684	Parallel	Bug 1816522
Confirm text style of tab items	Bug 1833493	Parallel	Bug 1826404
The collapsed synced tabs are expanded after switching from normal/private and back	Bug 1837417	Parallel	-
The spacing between the normal, private, and synced tabs is too small	Bug 1837984	Parallel	-
The normal browsing number of opened tabs has visual defects	Bug 1837986	Parallel	-
Inactive tabs should not be selectable in the tabs tray's multi-select mode	Bug 1838935	Parallel	-
Square button ripple effect is displayed on media button in Tabs Tray	Bug 1838140	Parallel	-
Investigate removing the config change flags from the manifest	Bug 1841909	Parallel	-
Elements from the previous/next tab view will be displayed after repeatedly changing the orientation	Bug 1837446	Sequential	Bug 1841909
The normal browsing number of opened tabs has a color flicker	Bug 1842223	Parallel	-

Do when time / not necessary for Beta

Task	Ticket	Sequential or parallel	Dependency
Refactor the stores into TabsTrayBanner	Bug 1833697	Parallel	-
Remove unnecessary parameter from tab list Composables	Bug 1833756	Parallel	-
Refactor TabsTrayBanner's onEnterMultiselectModeClick lambda parameter	Bug 1834844	Parallel	-
Consolidate tab click lambdas	Bug 1833757	Parallel	Bug 1833756
Refactor metrics out of TabsTrayFragment	Bug 1826152	Parallel	Bug 1814993
Refactor generateSingleSelectBannerMenuItems into an extension function and add test coverage	Bug 1833693	Parallel	-
Refactor the usages of observeAsComposableState to use the non-null variation	Bug 1834838	Parallel	-
Delete ComposeGridViewHolder and ComposeListViewHolder	Bug 1834890	Parallel	-
Move NormalTabsPage and PrivateTabsPage to their own files	Bug 1835900	Parallel	-
Rename SingleSelectBanner to TabPageBanner	Bug 1835898	Parallel	-
Rename SyncedTabs Compose file to match the Composable SyncedTabsList	Bug 1836354	Parallel	-

Phase 7 - Bake in beta

Task	Ticket
Enable in Beta	Bug 1815002

Phase 7.5 - Bugs found during Beta bake <----- We are here

All bugs found during the Beta bake are tracked in Bug 1841140

Phase 7 - Enable in Release

Task	Ticket
Beta to Release tasks	Bug 1881847
Enable in release	Bug 1815003

Phase 8 - Post release

Task	Ticket	Sequential or parallel	Dependency
Remove XML implementation	Bug 1815004	Sequential	Bug 1815003
Remove XML UI tests	Bug 1832620	Sequential	Bug 1815003
Swap out the HoriztonalPager component for the Compose library version	Bug 1819451	Parallel	-
Update the code verbiage of tab selection vs the currently active tab	Bug 1828527	Parallel	-
Refactor tab page button handler function	Bug 1835102	Parallel	Bug 1815004
Remove TabsTrayController.handleTabUnselected	Bug 1835123	Parallel	Bug 1815004

Phase 9 - Optional nice-to-haves

Task	Ticket	Sequential or parallel	Dependency
Animate the expanding/collapsing of the inactive tabs section	Bug 1824213	Parallel	Bug 1814993
Add TabsTrayFAB to the TabsTray previews	Bug 1826154	Parallel	Bug 1819196
Investigate integrating CompositionLocal with our Store pattern	Bug 1830416	Parallel	-

Comment 4

[Noah Bond [:007]]

Please file all bugs found during the Nightly bake as a blocker for Bug 1841141

Please file all bugs found during the Beta bake as a blocker for Bug 1841140

Comment 5

[Noah Bond [:007]]

Ideas for future enhancements should be filed under Bug 1881848.

Comment 6

[Noah Bond [:007]]

The bugs fixed by the rewrite can be tracked with the [fixed-by-tabs-tray-compose] whiteboard