Page MenuHomePhabricator

[XL] [Tech Debt] Standardize programmatic navigation
Open, LowPublic

Description

Background Information: Apps Debt

Our approach to navigation in the codebase is inconsistent and can be sourced from multiple different areas. We need to clean this up and ensure all navigation from within the app and via deep links happens via the same pattern, and that this path is heavily unit tested. We should also document supported deep links while we are here.

This work will pave the way for a smoother navigation restructuring that we plan to do later this year.

Our navigation approach occurs primarily in two different ways:

  1. Calling the standard navigationController?.pushViewController(viewController, animated:) and present(viewController, animated:) methods from the view controller we are navigating away from. This is what we want to move away from. It prevents us from easily reusing view controllers in other features.
  2. Creating an NSUserActivity. These activities are sometimes created directly or by calling the view controller extension navigate(to: URL), which in in turn fires an NSNotification that carries an NSUserActivity. That notification is picked up by the WMFAppController, where it either push/presents a view controller or sends it along further to the ViewControllerRouter, where it push/presents a view controller. The app also handles application deep links via NSUserActivity.

We should evaluate how the app is utilizing NSUserActivities for spotlight, state restoration, and handoff, and ensure they are only used (and used correctly) in those cases. Any unrelated navigation work should be implemented with an alternative method, like Coordinators.

  • Spike: Get a better handle on navigation patterns (Coordinators, anything else?), as well as where NSUserActivities for spotlight, state restoration, and handoff may fit into these patterns.
  • Prototype two flows in the app using the chosen navigation pattern: (1) On Explore feed, tapping "All top read articles", then tapping an article. (2) Deep linking from Safari to a particular article.
  • Add unit tests for the prototype flows. We will not add any more until navigation work is restructured later in the year.
  • Implement the prototype pattern everywhere in the app. Handle navigation occurring within:
    • Explore Feed
    • Places
    • Saved
    • History
    • Search
    • Settings
    • Article
    • Notifications Center
    • Talk pages
    • Editor Flow
    • Panels.swift modals
    • WMFAlertManager buttons
    • Navigation State restoration (as of this time, this should only restore their last read article rather than their entire navigation stack)
  • Update Deep Link documentation (T251929)

Event Timeline

JTannerWMF moved this task from Needs Triage to Up next on the Wikipedia-iOS-App-Backlog board.
Tsevener renamed this task from [Tech Debt] Standardize programmatic navigation to [XL] [Tech Debt] Standardize programmatic navigation.May 14 2024, 5:17 PM

Notes from engineering sync:

We marked as XL, but leaving in engineering sync so that we can break it down further. For now we will just add checkbox items to this task and work against it, but we can split it off further if needed later on.

We should do this work in a feature branch so that the new paradigm is used wholesale once we do the switch. This will prevent more navigation inconsistencies from creeping into the codebase.

Tsevener updated the task description. (Show Details)
Tsevener updated the task description. (Show Details)
Tsevener updated the task description. (Show Details)

Expecting just spike investigation for the moment. To go back into the backlog.

Spike findings:

The initial take on this task became a Spike in updating our navigation to a more flexible approach, untying this logic from the ViewControllers.

The concept that offers the flexibility to adapt our app, plus has a wealth of documentation online to refer from is the Coordinator pattern. The Coordinator pattern is a design pattern used in iOS applications to manage navigation and the flow of view controllers in a modular and organized way. It addresses the problem of view controllers becoming too complex by taking over the responsibility of navigation.
It usually depends on a Coordinator protocol that can be as simple as

protocol Coordinator {

func start()

}

and maybe a base coordinator class that can contain child coordinators.

Each part of the application would have its coordinator, which extends that protocol (or base coordinator.

Coordinators would be responsible for injecting the dependencies needed, improving the separation of concerns on the app, and improving the readability and maintainability of the code.
Most importantly, it allows us to implement it partially if necessary.

To implement it in our application, we would need to refactor WMF app view controller into a simpler TabBarController that is not responsible for creating its dependencies so that it can be instantiated by a Coordinator, like initializing the data store and restoring state.

Navigating to a specific part of the app with Widgets and deep links is also possible by triggering specific coordinators.

Other approaches experimented:
Using URL routers, but there was way less documentation online.
SwiftUI navigation links. The SwiftUI native navigation stack is similar to what we currently have and tightly coupled.

I'm not linking my branch because it stopped at a point were it was not working correctly.

Mazevedo subscribed.