refactor: swaps routes, closes #4317

[fbwoolf]

Try out Leather build 65b7a38 — Extension build, Test report, Storybook, Chromatic

This PR refactors the swaps routes so that now a user can see swap/:base/:quote as part of the url path. The form/path is loaded with swap/STX/:quote. Also, a user can now type in the route swap/STX/ALEX and the swap form will automatically select those tokens to swap.

To select an asset is: /swap/:base/:quote/select-base or /swap/:base/:quote/select-quote
To review the swap is: /swap/:base/:quote/review

I also did a bunch of renaming here to refactor away from using to and from, instead we are now using base and quote in the codebase naming convention for swaps.

[kyranjamie]

Nice improvement @fbwoolf !

Notice the default state is :quote. Could this instead be blank?

[fbwoolf]

Notice the default state is :quote. Could this instead be blank?

Let me check this, I bet it is possible.

[pete-watters]

I agree, this isn't great. Before we were storing the whole header including callback in the state.

Maybe we could try using the location state to store where it should go back to?

For some of the other dynamic routes I was using match to get to more specific cases.

If we use includes here it also means if you reach the /swap/review page then you go all the way back to the homepage on back and not to the previous page where you are selecting what to swap.

In those cases I think it should go back navigate(-1)

[kyranjamie]

Rather than trying to put all the go back logic in the container—which think we all agree isn't a scalable solution—can't we instead find a way to define this in each component tree where there is a back button?

What if the header back button simply publishes a "go back" event, e.g. with PubSub pattern, and components, via a hook listen for this event?

function useHandleGoBack (handler) {
  // listen for go back logic, then fire handler()
}
export function Swap() {
  // nav passed as handler to avoid useNavigate if not needed
  useHandleGoBack(navigate => navigate(RouteUrls.Home))
  // would be nice to make a friendly API for this fn, perhaps could take a callback _or_ a RouteUrl for simple cases
  useHandleGoBack(RouteUrls.Home)

Perhaps not perfect. We'd need to be careful not to have double handlers, and ensure we define goback where necessary, but this demonstrates a way we can completely decouple the logic.

Any other ideas?

[kyranjamie]

Opened issue for this here #5181

[pete-watters]

Another idea I have is to have route props that specify it. So we can pass a call back either when loading the component in the routes file, or have it as part of the location state. I'll have a think about it.

Getting the headers out of state was a good start but this logic isn't great so I am open to a full overhaul. If you check in route-helpers.ts, I've had to use routes to apply other logic so it could be good to also improve that

[fbwoolf]

Can you make an issue to refactor this based on your understanding for what you wrote here and your implementation? I will likely not tackle this here.

[fbwoolf]

Nvm, my comments hadn't refreshed, sorry that last comment was outdated.

[pete-watters]

Nice work @fbwoolf . I added some suggestions but the only real thing to fix is the initial :quote in the URL.

I noticed also that on the swap/review screen onBack goes all the way to the home page.

Lets think about how we can improve the handling of onBack in general as I agree the code in container.tsx isn't great and won't scale well