1979928 - Replace getter usage of `BrowsingModeManager.mode` to `AppStore.state.mode`

Status: ASSIGNED

(Firefox for Android :: Design System and Theming, task, P3)

Description

[Gabriel Luong [:gl] (ΦωΦ)]

In Bug 1923650, we have synchronized the AppState.mode with the BrowsingModeManager.mode. In Bug 1977790, we have ensured that the AppState.mode is always synchronized when the BrowsingModeManager.mode is initialized. While any plan to remove BrowsingModeManager is still quite far, we should confidently be able to read the BrowsingMode from AppState.mode going forward.

The task here to refactor BrowsingModeManager.mode getter calls to use AppStore.state.mode instead. This will have the added benefit of reducing one less dependency of HomeActivity to access BrowsingModeManager in certain cases. Ultimately, this helps us move incrementally towards deprecating BrowsingModeManager and removing dependency on HomeActivity.

Comment 1

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1979928 - Part 1: Use `AppState.mode` to get the browsing mode in `SessionControlController.handleShowWallpapersOnboardingDialog`

Comment 2

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1979928 - Part 2: Use `AppState.mode` to get the browsing mode in `MenuDialogFragment`

Comment 3

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1979928 - Part 3: Use `AppState.mode` to get the browsing mode in `AddonsManagementFragment`

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b0da66c1cfc7
https://hg.mozilla.org/integration/autoland/rev/a06af65fd6d4
Part 1: Use `AppState.mode` to get the browsing mode in `SessionControlController.handleShowWallpapersOnboardingDialog` r=android-reviewers,jonalmeida
https://github.com/mozilla-firefox/firefox/commit/6a4e3ab5a5d0
https://hg.mozilla.org/integration/autoland/rev/cfe7cfb29af2
Part 2: Use `AppState.mode` to get the browsing mode in `MenuDialogFragment` r=android-reviewers,jonalmeida
https://github.com/mozilla-firefox/firefox/commit/93ae5c28587b
https://hg.mozilla.org/integration/autoland/rev/fbbb7fc45983
Part 3: Use `AppState.mode` to get the browsing mode in `AddonsManagementFragment` r=android-reviewers,rpl,jonalmeida

Comment 5

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/a06af65fd6d4
https://hg.mozilla.org/mozilla-central/rev/cfe7cfb29af2
https://hg.mozilla.org/mozilla-central/rev/fbbb7fc45983

Comment 6

[Nick Alexander :nalexander [he/him]]

The general thrust of this ticket is to have various parts of Fenix determine the browsing mode from AppState. Fine; it's a fundamental design decision of the product and as such it's part of the application state.

However, as I investigate modularizing Fenix (i.e., splitting into multiple Gradle modules), the AppState is most naturally at the very top of the hierarchy, and the theming code is almost at the very bottom. That is, AppState aggregates everything, and everything depends on the theme.

But the theme needs to know the browsing mode, and in fact, the theme might even want to own the browsing mode; they're almost the same bit of information. This is a tension.

How do folks who have more context than do I see this evolving? Jon?

Comment 7

[Jonathan Almeida [:jonalmeida]]

I don't see the theme owning the mode, at least with how we see modes today because we have a special mode that is not related to the system, private mode. The theme is something that I see being feed with which colour palette we want to use when the system tells us "the sun is going down, switch to dark mode", but we override this request based on the selectedTab and user preference (are we in private mode when I click a toggle). If our selectedTab is private, we inform the the theme to use the private palette even if the system said to use the dark one.

If our theme owns the mode, we would then have to make the theme Smart so that we can tell it from different parts of the application which mode we're in.

Having the theme observe our global state to know what to change it to seems reasonable so that we can then see which caller (action) performed the mode change and not having to add break points in all the places that can talk to the theme.

I could have misunderstood your concern too. Please let me know!

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Firefox for Android::Design System and Theming' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.