Improved the progress bar animation . fix for discussion #8041

[joydeep-bhowmik]

The problem

The issue was with default progress bar disappearing halfway instead of completing the animation after fetching a page. The provided GIF comparisons illustrate the behavior of the progress bar in Livewire, Inertia, and the fixed version.

Livewire : (use this link to view this image if this gif glitching)

Inertia :

Improved :

Changes Made

1. bar.js

In the bar.js file located at plugins/navigate/bar.js, the following changes were made:

Before:

NProgress.configure({
    minimum: 0.1,
    trickleSpeed: 200,
    showSpinner: false,
})

...

export function finishAndHideProgressBar() {
    inProgress = false
    NProgress.done()
    NProgress.remove()

    // finishProgressBar(); destroyBar()
}

After:

NProgress.configure({
    minimum: 0.1,
    trickleSpeed: 200,
    showSpinner: false,
    parent: 'html',
})

...

export function finishAndHideProgressBar() {
    inProgress = false
    NProgress.done()

    setTimeout(() => {
        NProgress.remove()
    }, 400)

    // finishProgressBar(); destroyBar()
}

The modification involves appending the progress bar to the html element instead of the default body element. Additionally, a delay was introduced before removing the progress bar, ensuring it completes its animation before removal.

2. history.js

In the history.js file located at plugins/navigate/history.js, the following changes were made:

Before:

export function updateCurrentPageHtmlInHistoryStateForLaterBackButtonClicks() {
    // Create a history state entry for the initial page load.
    // (This is so later hitting back can restore this page).
    let url = new URL(window.location.href, document.baseURI)

    replaceUrl(url, document.documentElement.outerHTML)
}

After:

export function updateCurrentPageHtmlInHistoryStateForLaterBackButtonClicks() {
    // Create a history state entry for the initial page load.
    // (This is so later hitting back can restore this page).
    let url = new URL(window.location.href, document.baseURI)

    const clonedDocumentElement = document.documentElement.cloneNode(true)

    const progress = clonedDocumentElement.querySelector('#nprogress')
    // Removing the progress bar before saving the HTML.
    progress && progress.remove()

    replaceUrl(url, clonedDocumentElement.outerHTML)
    // Removing the cloned element from memory.
    clonedDocumentElement.remove()
}

Here, instead of directly saving document.documentElement, a clone is created and the progress bar is removed from it before saving the outer HTML. Finally, the cloned element is removed from memory.

[calebporzio]

Nice! Thank you for the wonderful PR description. Made it really easy to grok.

Couple thoughts about this bit:

    const clonedDocumentElement = document.documentElement.cloneNode(true)

    const progress = clonedDocumentElement.querySelector('#nprogress')
    // Removing the progress bar before saving the HTML.
    progress && progress.remove()

First thought: Is cloning the entire document a bad idea performance wise? I know it gets GC'd, but it's often a pretty giant node to clone. Probably nothing, just want to make sure it doesn't take more than a couple ms.

Second thought: I wonder if we can skip that whole part of we make a memo of the state of the HTML when the navigate link is clicked instead of when the response is received?

This way we would have the document without the progress bar in it.

It might actually solve some kinda-related problems with loading indicators if we take the HTML snapshot a bit before the request goes out.

What do you think?

[joydeep-bhowmik]

Thank you for your response

Second thought: I wonder if we can skip that whole part of we make a memo of the state of the HTML when the navigate link is clicked instead of when the response is received?

that's so much better.

so we keep this as it is

//plugins/navigate/history.js
export function updateCurrentPageHtmlInHistoryStateForLaterBackButtonClicks() {
    // Create a history state entry for the initial page load.
    // (This is so later hitting back can restore this page).
    let url = new URL(window.location.href, document.baseURI)

    replaceUrl(url, document.documentElement.outerHTML)
}

and in plugins/navigate/index.js instead of this

function navigateTo(destination, shouldPushToHistoryState = true) {
        
        showProgressBar && showAndStartProgressBar()

        fetchHtmlOrUsePrefetchedHtml(destination, (html, finalDestination) => {
            fireEventForOtherLibariesToHookInto('alpine:navigating')

            restoreScroll && storeScrollInformationInHtmlBeforeNavigatingAway() //1

            showProgressBar && finishAndHideProgressBar()

            cleanupAlpineElementsOnThePageThatArentInsideAPersistedElement() //2

            updateCurrentPageHtmlInHistoryStateForLaterBackButtonClicks() //3

            //....

             }
            );
    }

we can do

 function navigateTo(destination, shouldPushToHistoryState = true) {
            restoreScroll && storeScrollInformationInHtmlBeforeNavigatingAway(); //1

            cleanupAlpineElementsOnThePageThatArentInsideAPersistedElement(); //2

            updateCurrentPageHtmlInHistoryStateForLaterBackButtonClicks(); //3

           showProgressBar && showAndStartProgressBar()

          fetchHtmlOrUsePrefetchedHtml(destination, (html, finalDestination) => {
              fireEventForOtherLibariesToHookInto('alpine:navigating')

              showProgressBar && finishAndHideProgressBar()

              //....

             }
            );
    }
}

so we will be saving the snapshot just before starting the navigation process and progress bar

[calebporzio]

@joydeep-bhowmik - ok, I did some digging on this and found that all I needed to do was make the parent: 'html' change for it all to work.

I updated this PR to reflect that.

I'm wondering if this is actually good enough and we can drop all the other changes with re-ordering the lifecycle and cloning and what not.

Let me know!

[joydeep-bhowmik]

Actually, the problem lies within this code snippet:

// js\plugins\navigate\bar.js
export function finishAndHideProgressBar() {
    inProgress = false;
    NProgress.done();
    NProgress.remove();

}

The NProgress.remove() call removes the progress bar before the done() function completes the animation. We can fix this by introducing a delay:

setTimeout(() => {
    NProgress.remove();
}, 400);

but, this results in saving the progress bar in the snapshot. So, one solution is to save the snapshot before the progress element renders. The parent: html is just to ensure that the done animation isn't interrupted when the body element is updating.

but I think I was doing it wrong .

re-ordering the lifecycle and cloning

I have a better solution in my mind which will not include any of this . let me check if it works.

[joydeep-bhowmik]

So I looked in to documentation of Nprogress

$(document).on('page:fetch',   function() { NProgress.start(); });
$(document).on('page:change',  function() { NProgress.done(); });
$(document).on('page:restore', function() { NProgress.remove(); });

and found out that we just needed to remove Nprogress when a cached page is restored.

so the current changes are just 3 lines

//js\plugins\navigate\bar.js
NProgress.configure({
    minimum: 0.1,
    trickleSpeed: 200,
    showSpinner: false,
    parent: 'html' // 1
})
//...

export function finishAndHideProgressBar() {
    inProgress = false
    NProgress.done()
    // NProgress.remove() // 2
    // finishProgressBar(); destroyBar()
}

and

//js\plugins\navigate\history.js
export function whenTheBackOrForwardButtonIsClicked(
    registerFallback,
    handleHtml
) {
         //...

        if (snapshotCache.has(alpine.snapshotIdx)) {
            let snapshot = snapshotCache.retrieve(alpine.snapshotIdx)

            handleHtml(snapshot.html, snapshotCache.currentUrl, snapshotCache.currentKey)

            NProgress.remove();// 3
        } 
        
         //...
    })
}

and its working fine.

[calebporzio]

Ah I see, that all makes sense. It's also inline with how they recommend you use nProgress with turbolinks:

Link for the above

Ok, I refactored and am ready to merge. Thanks for the help.

[joydeep-bhowmik]

glad to help. :)