Skip to content

Commit

Permalink
Replace when navigating to the same url
Browse files Browse the repository at this point in the history
  • Loading branch information
@lazaronixon
lazaronixon committed Sep 12, 2023
1 parent c295549 commit abcecea
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 3 deletions.
9 changes: 7 additions & 2 deletions ...alizer/templates/ios_stack/TurboNativeProject/Controllers/TurboNavigationController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,14 @@ class TurboNavigationController : UINavigationController {
}

// - Create view controller appropriate for url/properties
// - Navigate to that with the correct presentation
let viewController = makeViewController(for: url, properties: properties)
navigate(to: viewController, action: options.action, properties: properties)

// - Navigate to that with the correct presentation
if session.activeVisitable?.visitableURL != url {

This comment has been minimized.

Sign in to view

Copy link
@joemasilotti

joemasilotti Sep 12, 2023

You might want to think about integrating Turbo Navigator as there are a few more edge cases that could be handled!

This comment has been minimized.

Sign in to view

Copy link
@lazaronixon

lazaronixon Sep 13, 2023

Author Owner

Hey @joemasilotti, I tried Turbo Navigator in the beginning. Some points that made me choose this path:

So as soon we have Turbo Navigator merged, 37signal using it and the demo updated I would be happy to implement it. Thank you for all the work on turbo native, many stuffs here are from your articles including this one.

This comment has been minimized.

Sign in to view

Copy link
@joemasilotti

joemasilotti Sep 13, 2023

Turbo Navigator is planned to be upstreamed into turbo-ios eventually. But a few rough edges need to be worked out. So your feedback is super valuable!

I wasn't happy with the global configuration and was afraid it would be harder to customize.

Would love to hear more on this. What about it being global made it hard to customize?

I wanted to follow more the 37signal way.

Both path configurations you link to include a bunch of modals and stuff that Turbo Navigator handles. Including flows that turbo-android already handles out of the box. Can you share more on what you mean by this?

This comment has been minimized.

Sign in to view

Copy link
@lazaronixon

lazaronixon Sep 13, 2023 •
edited
Loading

Author Owner

Would love to hear more on this. What about it being global made it hard to customize?

Probably it was a false assumption from my side, I was worried about implementing multiple tabs and customizing the viewController used for navigations. I was just worried about it blocking some customization.

Both path configurations you link to include a bunch of modals and stuff that Turbo Navigator handles. Including flows that turbo-android already handles out of the box. Can you share more on what you mean by this?

The only modal navigation I can see in Hey is in the /accounts, which in this case is not even a presentation modal, By the way, it is usually presentation not a context. This screen seems to be presented natively not using the path configuration and it has its own navigation/session. All the other modals seem to use this property always-dismiss=true, they're two navigations that keep it false, it's not clear to me if it is the modal navigation, but I tend to believe that they don't implement modal navigations, only show modals with always-dimiss.

This comment has been minimized.

Sign in to view

Copy link
@joemasilotti

joemasilotti Sep 13, 2023

Got it. If you run into a concrete issue let me know. I still haven't found a reason to make it non-global (even though I originally wanted to).

Looking at the path configuration you linked I see 20+ different rules that route to a modal presentation. And poking around the HEY app I can confirm these screens are being presented modally. There are even nested stacks (modal to another modal) that I could confirm in HEY.

This comment has been minimized.

Sign in to view

Copy link
@lazaronixon

lazaronixon Sep 13, 2023

Author Owner

Oh, which screen navigates inside modals?

This comment has been minimized.

Sign in to view

Copy link
@joemasilotti

joemasilotti Sep 13, 2023

If I click my avatar in HEY then Accounts & Settings it remains in the modal. Turbo Navigator handles this flow: if you are in a modal and route another modal it pushes in the modal context instead of dismissing then presenting.

This comment has been minimized.

Sign in to view

Copy link
@lazaronixon

lazaronixon Sep 13, 2023 •
edited
Loading

Author Owner

As I told it seems the accounts screen is presented modally from the native screen, it's not using the path configuration and it has its own session and modalSession.

image

This comment has been minimized.

Sign in to view

Copy link
@joemasilotti

joemasilotti Sep 13, 2023

I'm not sure I follow, how does that change anything?

This comment has been minimized.

Sign in to view

Copy link
@lazaronixon

lazaronixon Sep 13, 2023

Author Owner

I mean, maybe 37signal doesn't implement "context: modal" navigation using the configuration path, their default navigation controller only supports auto-dismiss modals.

This comment has been minimized.

Sign in to view

Copy link
@lazaronixon

lazaronixon Sep 13, 2023 •
edited
Loading

Author Owner

This is why I would like to see turbo navigation, merged with tubo-ios, because we would have those opinions aligned. Even though I see turbo-navigation tries to align turbo-android and turbo-ios, I don't know if 37signal expects the same.
navigate(to: viewController, action: options.action, properties: properties)
} else {
navigate(to: viewController, action: .replace, properties: properties)
}

// Initiate the visit with Turbo
if isVisitable(properties) {
Expand Down
8 changes: 7 additions & 1 deletion ...ializer/templates/ios_tabs/TurboNativeProject/Controllers/TurboNavigationController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ class TurboNavigationController : UINavigationController {
// - Create view controller appropriate for url/properties
// - Navigate to that with the correct presentation
let viewController = makeViewController(for: url, properties: properties)
navigate(to: viewController, action: options.action, properties: properties)

// - Navigate to that with the correct presentation
if session.activeVisitable?.visitableURL != url {
navigate(to: viewController, action: options.action, properties: properties)
} else {
navigate(to: viewController, action: .replace, properties: properties)
}

// Initiate the visit with Turbo
if isVisitable(properties) {
Expand Down

0 comments on commit abcecea

Please sign in to comment.