Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mapbox Navigation iOS Architecture Plan #1432

Closed
5 tasks
JThramer opened this issue May 14, 2018 · 1 comment
Closed
5 tasks

Mapbox Navigation iOS Architecture Plan #1432

JThramer opened this issue May 14, 2018 · 1 comment
Assignees
Labels
topic: architecture UI Work related to visual components, Android Auto, Camera, 3D, voice, etc.

Comments

@JThramer
Copy link
Contributor

JThramer commented May 14, 2018

(Revision: 1 - Words are hard.)

Hey folks,

This is a ticket I made to track the action items that came out of the architecture meeting between @frederoni and myself. This is definitely first-draft, and I fully expect this document to evolve as we discuss solutions. As we progress, ideally many of these points will be broken off into their own detailed stories, but this way we can get eyes on the overall plan and start the discussion.

The Plan:

  • 1. Move the StepsViewController to a proper view (ideally logically in the code that manages the top-banner stack) that manages it’s bounds properly — the way it is setup right now in the RouteMapViewController is weird.

  • 2. Move the EndOfRouteViewController out of storyboard and into code -- It's the last UIViewController standing.

  • 3. The “Migration” - Move code into appropriate classes — for instance, RouteMapViewController controls way too much right now, it should really only be concerned with the map itself and UI elements that are “on” the map. This is probably going to clutter up the NVC a little bit with extra properties, but it will temporarily reinforce separation-of-concerns and sets us up to put these new NVC properties in containers in the next step below. Part of the containerization plan is to write code that swaps out one contained VC with another, which is not trivial. Us migrating code before we rearchitect the infrastructure makes that re-architecture much easier because the code is “roughly in the right place.”

  • 4. The “Rearchitecting” - Setup “Container” View-Controller pattern for top/bottom bars, bringing advantage of high cohesion / loose coupling, and will open up a mechanism to do all UI transitions as swap outs of containers. View containers are desirable because they allow us to capture messaging for container size changes (exposed as preferredContentSize changes)

  • 5. The “De-delegation” - Delegation is a great pattern, but right now we implement it in an ugly way — we do a ton of manual message passing between the NVC and the various sub-components of the app. It would be really nice if we used delegate composition (example: protocol NavigationViewControllerDelegate: TopBannerDelegate, RouteMapViewControllerDelegate, BottomBannerDelegate) so we can pass a single delegate reference to the subcomponents, instead of manually routing messages.

FAQ’s:

  • Q: What sort of stuff should be migrated out of the RMVC into the NVC?
    A: If it’s not the map or an on-map button, it shouldn’t be in the RMVC.

Open-ended Q’s:

  • How will transitions between states work? We could define a monolithic controller with enumerated “states”, or we could whip up custom segues that define payloads for the “top” and “bottom” banners, ideally it would always be “stuff on top of the map swapping in and out”, the context-switch shouldn’t be rough because you’ll “always have the map in the middle”

/cc @akitchen @brsbl @mapbox/navigation-ios

@JThramer JThramer added epic topic: architecture UI Work related to visual components, Android Auto, Camera, 3D, voice, etc. labels May 14, 2018
@JThramer JThramer self-assigned this May 14, 2018
@akitchen
Copy link
Contributor

@JThramer can this be closed out in favor of #1602 and friends? I believe the plan has evolved significantly since this was written.

I guess re-open if not; closing for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: architecture UI Work related to visual components, Android Auto, Camera, 3D, voice, etc.
Projects
None yet
Development

No branches or pull requests

2 participants