[✨] Critical DX/UX improvement to SPA navigation in Qwik City

[billykwok]

What is it?

• Feature / Enhancement
• Bug / Fixes
• Docs / tests

Overview

This is a relatively straightforward change that addresses a number of interrelated issues around SPA navigation in Qwik City. Some are already mentioned in other issues (#2162, #2050, #3149, #2202, #2203), while some are newly reported here.

Support MPA-like and customizable scroll restoration in SPA

Before: Qwik City always scrolls to the top upon SPA navigation, unless there is a hash segment in the URL. This is not consistent with how browsers handle MPA scroll restoration on popstate and creates bad UX when users return to a page with a resetted scroll position. Moreover, the behavior is hard-coded into the Qwik City navigation logic, even though different applications have their own needs (e.g. app-like and page-like UIs should restore scroll positions differently).

After: The default behavior now mimics how browsers handle scroll restoration in MPA - scroll to the top for new pages (e.g. <Link />), but to the last-visited scroll position for popstate navigation (e.g. back/next button). Moreover, users can customize scroll restoration with the built-in scroll restorer toTopAlways and toLastPositionOnPopState or even their own implementation. The 'before' behavior is equivalent to toTopAlways.

• This provides feature parity against React Router and Remix
• This provides customizability for users to implement Adding and removing query params from url with useNavigate scrolls to top #2162 in userland

Race condition when restoring scroll positions

Before: Scroll position is restored after the DOM is 'settled' in order to synchronize content update and scroll position update. However, the current implementation relies on a series of hard-coded setTimeout(), which very often results in either early scrolling or late scrolling.

After: 'DOM Settled' is accurately detected using useLocation() and useVisibleTask$(). Scroll and content updates are now always in-sync.

• This provides feature parity against React Router and Remix
• This fixes fix(client-navigate.ts): Wait to scroll the page until navigation has finished #2050 and [🐞] actions from the Form are vanished in the history. #3149

Prevent full-page reload when popstate happens before manual navigation

Before: Qwik City registers a temporary popstate event listener to reload the page if there is any popstate navigation before the first pushState is called. This essentially turns all SPAs into MPA whenever users refresh the page. This also makes page navigation inconsistent before and after the first pushState. In addition, there are unnecessary popstate listener registration/unregistration, because the temporary listener will be replaced by a real listener anyway after the first pushState.

After: Instead of a temporary popstate listener that does full-page refreshes, Qwik City now directly sets up the real popstate listener that updates the page without full-page refreshes.

• This makes Qwik City consistent with all other SPA frameworks in the market.
• This also makes page transition animation across 'popstate' possible.

Improve hash navigations

Before: Hash navigation is neither updating the URL hash nor written into the session history. The 'hashchange' event is also never fired.

After: The new behavior now mimics native behavior - clicking <a href="#some-hash" /> now updates the page URL hash without triggering unnecessary re-rendering. It also gets written into session history like what browsers would natively do. The 'hashchange' event is also fired.

• This fixes the hash issue mentioned in feat: View Transition API #3691.

Backward-compatible API Changes

const navigate = useNavigate();
// New signature that accepts an options object
type Options = { type: 'form' | 'link' | 'popstate'; forceReload: boolean; };
const defaultOptions: Options = { type: 'link', forceReload: false };
navigate('/some/pathname', defaultOptions);

// Old signature still works
navigate('/some/pathname', true /* equivalent { forceReload: true } */);
navigate('/some/pathname', false /* equivalent { forceReload: false } */);

import {
  QwikCityProvider,
  RouterOutlet,
  ServiceWorkerRegister,
  toTopAlways
} from '@builder.io/qwik-city';
import { RouterHead } from './components/router-head/router-head';

export default function Root() {
  return (
    <QwikCityProvider>
      <head>
        <meta charSet="utf-8" />
        <link rel="manifest" href="/manifest.json" />
        <RouterHead />
      </head>
      <body>
        //            ⬇️ New (optional): toTopAlways or toLastPositionOnPopState (default)
        <RouterOutlet restoreScroll$={toTopAlways} />
        <ServiceWorkerRegister />
      </body>
    </QwikCityProvider>
  );
}

Demo

Scroll restoration after popstate

Before the change:

• always to the top

CleanShot.2023-03-18.at.01.07.05.mp4

After the change:

• scroll to the last-visited positions

CleanShot.2023-03-18.at.00.57.29.mp4

popstate after reload

Before the change:

• full-page reload the second time

CleanShot.2023-03-18.at.01.07.49.mp4

After the change:

• re-rendering

CleanShot.2023-03-18.at.01.38.36.mp4

Checklist

• My code follows the developer guidelines of this project
• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• Added new tests to cover the fix / functionality

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[zanettin]

Hi @billykwok 👋 thank you very much for creating this PR 🙏

I just have a question based on the description:

Currently, pages are always scrolled to the top (or hashed element if specified in URLs) upon navigation. While this makes sense for -triggered navigation, popstate-triggered navigation fails to restore previous scroll positions. This does not match the native behavior of browsers nor that of competing frameworks.

you mention, that the restore for MPA (popstate) navigations does not work atm, which i can't confirm so far. At least not on the available browsers 😄 The <Link> component driven navigations are the SPA (pushstate) ones which you also show in the video.

MPA navigation scroll position restore

Mar-03-2023.13-13-31.mp4

Is the description wrong in that case or do i miss something? 👼
Thanks again for your time and contribution 🙏

[billykwok]

Thanks for checking out my change. It is intended for SPA scrolling only. When I said popstate, I was referring to popstate navigation in SPA (e.g. same-page re-rendering due to pressing back button after pushState). MPA does not need manual scroll restoration because browsers handle it by default. Hope this clarifies the change.

[billykwok]

Converted to draft for now as I found a new way to implement the change that is more flexible and robust.
Will update it with the new implementation in the coming days.

[billykwok]

I just pushed an update to address all of the comments so far.

• There is now an even simpler way to detect DOM settlement accurately based on just useLocation() and useVisibleTask$(). The MutationObserver is no longer there.
• The hash issue mentioned in feat: View Transition API #3691 is now addressed.
• I removed the proposed 'qinit'/'qsettled' event API to reduce the scope.

@manucorporat Thank you for spending time looking at it previously. Hope you manage to squeeze some time on it again.

[manucorporat]

I want to merge this PR, but there are so problems, the thing i am more against is the usage of useVisibleTask and useWindowOn() in RouterOutlet, right now, QwikCity apps download no js, adding this changes the baseline, which affects demos and simple apps.

[billykwok]

Thanks for following up. I appreciate the pursuit of avoiding the initial JS load at all costs. However, there is probably no simple way to achieve the intended result of this PR with Qwik's current design/APIs.

While I agree that the current implementation may not fully align with Qwik's vision, the UX issues in scroll restoration mentioned in the original post should still be fixed ASAP. It is not desirable and hurts adoption if this is left hanging. There might be some clever tricks that I'm not aware of (perhaps through some unknown internal APIs). Unfortunately, I've decided to move on from this.

For the hard refresh issue specifically, it may not be that big of a concern after all. Since Qwik embraces and optimizes for MPA more than most JS frameworks (except Astro), maybe triggering a hard page refresh on popstate is acceptable for most Qwik adopters (unfortunately I am not among this crowd).

Qwik has introduced so many innovative solutions to improve the initial page load experience, and it has so much potential IMO. I am happy to revisit it one day when it is more compatible with SPA. But until then, if someone cares enough about the SPA Scrolling UX in Qwik and has a solution that fits into Qwik's vision, please feel free to pick this up and fix it once and for all.

I am closing this PR for now. Thank everyone who has put time into it.

[manucorporat]

Hey! actually i am working on it, i think it's possible

[manucorporat]

We will get it merged!

[manucorporat]

Just pushed a bunch of changes! can you review and get your thoughts? @billykwok

[manucorporat]

Do you think we could merge and iterate, seems like this PR would fix some issues + add a cool feature, what if we leave for a second PR:

• Fix flickering
• Fix the popstate (very tricky one)

[manucorporat]

This PR tried to do many things at the same time and it makes it very hard to merge, might be a good moment to merge and iterate

[manucorporat]

let me try to find him in discord!

[manucorporat]

Let's do it! merged this for the 1.2.0 window! cc @jordanw66

[thejackshelton]

@billykwok @jordanw66 @manucorporat this is INCREDIBLE. Thank you guys so much for the contributions!!