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

Interaction between the stop button and navigate event #38

Closed
pshrmn opened this issue Feb 13, 2021 · 4 comments · Fixed by #68
Closed

Interaction between the stop button and navigate event #38

pshrmn opened this issue Feb 13, 2021 · 4 comments · Fixed by #68
Labels
addition A proposed addition which could be added later without impacting the rest of the API

Comments

@pshrmn
Copy link

pshrmn commented Feb 13, 2021

One of the biggest issues that I ran into with my attempts to implement asynchronous navigation on top of the existing history API is that there was not a browser-based method for cancelling an asynchronous navigation. For me, the ideal mechanism to do this would be for the browser to to swap the refresh button to a stop button like it does when navigating to another origin.

It gives the browser clear insight into when a navigation is being handled as a single-page navigation, and the provided promise allows the browser to know how long the navigation takes, and whether or not it succeeds. We expect browsers to use these to update their own UI (including any loading indicators; see whatwg/fetch#19 and whatwg/html#330 for previous feature requests).

Does this expectation include showing a stop button that would reject the navigation promise or is it purely for showing a loading bar/spinner?

@domenic
Copy link
Collaborator

domenic commented Feb 16, 2021

This is a great question. I think this would be a very reasonable thing for browsers to do. However, there are some questions about how it would integrate with the navigation.

Just rejecting the navigation promise is probably not enough, because the page will still continue to do its async work. I.e.

appHistory.addEventListener("navigate", e => {
  e.respondWith((async () => {
    const res = await fetch(e.destination.url);
    const body = await res.body.text();
    document.body.innerHTML = body;
  })());
});

try {
  await appHistory.push({ url: "/foo" });
  // User pushes stop while the above event listener is running...
} catch (e) {
  // OK, we rejected the promise, but everything inside the event listener still happened.
  // In particular, innerHTML got updated.
}

I think the best design would be to hand the event listener an abort signal. So, you'd implement something like:

appHistory.addEventListener("navigate", e => {
  e.respondWith((async () => {
    const res = await fetch(e.destination.url, { signal: e.signal });
    const body = await res.body.text();
    document.body.innerHTML = body;
  })());
});

try {
  await appHistory.push({ url: "/foo" });
  // User pushes stop while the above event listener is running...
} catch (e) {
  // The AbortSignal fired its abort event, so if the fetch() or text() call was running, it got aborted.
  // So, innerHTML did not get updated!
  // Then it threw an "AbortError" DOMException which was naturally propagated to here.
}

Note that this does allow the navigate handler to totally ignore the stop request from the browser, by just not using the signal property. But that seems OK to me. WDYT?

@domenic
Copy link
Collaborator

domenic commented Feb 17, 2021

One tweak might be requiring that the page signal whether it plans to handle the stop button, e.g. e.respondWith(promise, { canHandleAbort: true }). If that isn't set then the browser would avoid showing the stop button (maybe disable it, maybe hide it, maybe leave it as a reload button...).

@domenic domenic changed the title Clarification on browser UI updates during asynchronous navigations Interaction between the stop button and navigate event Feb 17, 2021
@domenic domenic added the addition A proposed addition which could be added later without impacting the rest of the API label Feb 17, 2021
@pshrmn
Copy link
Author

pshrmn commented Feb 18, 2021

It is definitely important that an application is able to detect when a navigation has been cancelled. The pattern that I had ended up with is that at the start of a navigation event you would get a pending navigation object. The router would emit this pending navigation and could trigger a cancellation event that would modify the pending navigation to indicate that it was cancelled. During the navigation, the router would run any asynchronous code and when it resolved, check if the pending navigation had been cancelled before updating the UI. I believe that that is generally analogous to using an abort controller, so I agree that that pattern would work.

I think that the case where the app doesn't allow the stop button is interesting. I don't see a good reason why an app shouldn't let a user cancel a navigation, but maybe this would just be the case where it isn't "smart" enough to know what to do. In theory, I like the idea of a user always being able to cancel the navigation, but considering that the mobile use case requires opening a submenu to even access the stop button, perhaps it is a second class citizen.

@frehner
Copy link

frehner commented Feb 28, 2021

An abort signal was added to the navigation event in #46

domenic added a commit that referenced this issue Mar 11, 2021
Closes #38 by making it explicit that the stop button is meant to trigger the navigate event's AbortSignal.

Closes #10 by getting rid of queued-up navigations, their associated event, and their associated callback variants to update() and push(). Instead, navigations during other navigations interrupt the ongoing one. As the new example tries to show, this seems to work pretty well.

Closes #13 by canceling all pending navigations when another one starts, of which the case discussed there is one example.
domenic added a commit that referenced this issue Mar 11, 2021
Closes #38 by making it explicit that the stop button is meant to trigger the navigate event's AbortSignal.

Closes #10 by getting rid of queued-up navigations, their associated event, and their associated callback variants to update() and push(). Instead, navigations during other navigations interrupt the ongoing one. As the new example tries to show, this seems to work pretty well.

Closes #13 by canceling all pending navigations when another one starts, of which the case discussed there is one example.
domenic added a commit that referenced this issue Mar 16, 2021
Closes #38 by making it explicit that the stop button is meant to trigger the navigate event's AbortSignal.

Closes #10 by getting rid of queued-up navigations, their associated event, and their associated callback variants to update() and push(). Instead, navigations during other navigations interrupt the ongoing one. As the new example tries to show, this seems to work pretty well.

Closes #13 by canceling all pending navigations when another one starts, of which the case discussed there is one example.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition A proposed addition which could be added later without impacting the rest of the API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants