forXifLess / LinkNavigator

🌊 Easy & Powerful navigation library in SwiftUI
Other
332 stars 24 forks source link

Need more consideration about when the same matchingKeys are appearing #8

Closed jay-handhug closed 1 year ago

jay-handhug commented 1 year ago

Hello there. I think i found some cases LinkNavigator cannot cover which are quite common and i wish you guys to support it so that it can be more ergonomic library.

The problem is occurring when the same machingKeys are appeaing in the currentPaths. Suppose that currentPaths is ["A", "B", "A", "B", "A", "B"] and i want to remove last two paths so it to be ["A", "B", "A", "B"]. First Intuition, i thought 3 options and it all doesn't satisfy my requirements.

I think you guys need more consideration about the situation i mentioned, and i hope the library needs to support removeLast(n) mechanism at least.

jaeho-flitto commented 1 year ago

Hi @jay-handhug, thanks for your proposal.

The situation you elaborated, stacking pages that have the same matchPath, is not a common use case. LinkNavigator enables accurate navigation through a unique 'matchPath' assigned to each page.

the method removeLast(n) is a great concept, but removing n pages from the navigation stack seems risky and error-prone in a complex app.

How about using replace(paths:items:isAnimated:) to make currentPaths with duplicated pages?

jay-handhug commented 1 year ago

Appreciate for your replying @jaeho-flitto.

First, i don't agree that it's not a common use case, because you can see it quite easily in places where the app has user profile page like Instagram, Airbnb. Second, you mentioned 'replace' method but it re-builds all views and i don't want it to be happened becuase it's not efficient and all the contexts will blows away.

I agree that removeLast(n) mechanism is not a safe way but we can make it more safe by doing like removeLast(paths: [String])

interactord commented 1 year ago

Thank you for your interest in and suggestions for our library. @jay-handhug

The particular app case we mentioned is a different concept than our library. If we were to develop or manage an app with that concept, we should have developed it in a different way. In the case of overlapping matchPath in the way you suggested, it is absolutely not in line with our intention.

Perhaps, you judge that you know about it. The intention of developing the library is based on matchPath to integrate navigation movement, DeepLink, and push movement system.

First of all, the function you suggested was added to version 0.5.2.

interactord commented 1 year ago

I would be very grateful if you consider PR when you make a proposal from now on.