designly1 / nextjs13-progress

An implementation of n-progress for use with the Next.js 13 app router.
MIT License
13 stars 5 forks source link

Conditionally prevent progress bar Link's onClick firing #1

Closed gamma613 closed 9 months ago

gamma613 commented 9 months ago

I am wondering whether there's a way to prevent or disrupt the onClick of the Link component, which kicks off the progress bar

For greater context, I'm adding an additional onClick function to the Link. This function uses the browser's dialog to prompts the user if they have unsaved changes in a form. The prompt comes up before the progress bar starts, which seems promising. When they click "cancel", my custom onClick function runs event.preventDefault(), which successfully stops the navigation, but the progress bar fires anyway. I've also tried other things like event.stopPropagation or modifying the event, but to no effect.

Thanks for this project, BTW. It's been a lifesaver.

gamma613 commented 9 months ago

I worked on this some more, and realized that my custom event does fire second, as your code would indicate. The progress bar (and all other functionality) seemed to be suspended by the browser's dialog box.

I did manage to find an imperfect workaround, by running NProgress.done(true) after my e.PreventDefault(). That at least keeps the progress bar from continuing forever, but ideally I'd like to prevent it starting at all.

designly1 commented 9 months ago

Thank you for your feedback!

Why not try something like this:

import React from 'react'; import { useRouter } from 'nextjs13-progress';

export default function MyComponent() { const router = useRouter();

const confirmNavigation = () => {
    // TODO: implement confirm dialog
    return true;
};

const handleClick = () => {
    if (confirmNavigation()) {
        router.push('/some/path');
    }
};

return <a onClick={handleClick}>My Link</a>;

}


From: Steven McCoy @.> Sent: Wednesday, December 20, 2023 2:42 PM To: designly1/nextjs13-progress @.> Cc: Subscribed @.***> Subject: [designly1/nextjs13-progress] Conditionally prevent progress bar Link's onClick firing (Issue #1)

I am wondering whether there's a way to prevent or disrupt the onClick of the Link component, which kicks off the progress bar

For greater context, I'm adding an additional onClick function to the Link. This function uses the browser's dialog to prompts the user if they have unsaved changes in a form. The prompt comes up before the progress bar starts, which seems promising. When they click "cancel", my custom onClick function runs event.preventDefault(), which successfully stops the navigation, but the progress bar fires anyway. I've also tried other things like event.stopPropagation or modifying the event, but to no effect.

Thanks for this project, BTW. It's been a lifesaver.

— Reply to this email directly, view it on GitHubhttps://github.com/designly1/nextjs13-progress/issues/1, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALHCNZCDIO7NTLZAXP2EYFLYKNERRAVCNFSM6AAAAABA5MS3D6VHI2DSMVQWIX3LMV43ASLTON2WKOZSGA2TCMRTHA4TMNY. You are receiving this because you are subscribed to this thread.Message ID: @.***>

gamma613 commented 9 months ago

I appreciate the idea, but my implementation has custom handlers for both the Link component and UseRouter hook, which listen for a changed values - that way it can catch router events and link clicks across the whole site

gamma613 commented 9 months ago

I'm thinking more toward the following section of the code (inlink.tsx) and wondering whether it would make sense to fire any existing existing onClick handler first, given that it might be intended to conditionally preventDefault, and in which case we wouldn't want to run the progress bar onStart().

            onClick={event => {
                if (shouldTriggerStartEvent(href, event)) onStart();
                if (onClick) onClick(event);
            }}

Although I could also see use cases where we'd want the existing onClick to fire secondly. Maybe the solution would be to:

  1. Export / expose shouldTriggerStartEvent(), onStart() and onComplete() so that folks can write their own, custom onClick handler with full control over the chain of events
  2. Add a parameter to the nextjs13-progress <Link/> to supply this custom onClick event without it being overridden. Ex: <Link ref={ref} onClickImmutable={myFunctionThatIncludesOnStart} {...other} />
designly1 commented 9 months ago

Sorry, I misunderstood what you were suggesting. I have no problem exposing more of the API. Did you want to go ahead and fork and submit a PR or I can probably get to it this weekend. Lmk. 🙂

Jay


From: Steven McCoy @.> Sent: Thursday, December 21, 2023 8:05 AM To: designly1/nextjs13-progress @.> Cc: Jay Simons @.>; Comment @.> Subject: Re: [designly1/nextjs13-progress] Conditionally prevent progress bar Link's onClick firing (Issue #1)

I'm thinking more toward the following section of the code (inlink.tsx) and wondering whether it would make sense to fire any existing existing onClick handler first, given that it might be intended to conditionally preventDefault, and in which case we wouldn't want to run the progress bar onStart().

                            if (shouldTriggerStartEvent(href, event)) onStart();
                            if (onClick) onClick(event);
                    }}

Although I could also see use cases where we'd want the existing onClick to fire secondly. Maybe the solution would be to:

  1. Export / expose shouldTriggerStartEvent(), onStart() and onComplete() so that folks can write their own, custom onClick handler with full control over the chain of events
  2. Add a parameter to the nextjs13-progress to supply this custom onClick event without it being overridden. Ex: <Link ref={ref} onClickImmutable={myFunctionThatIncludesOnStart} {...other} />

— Reply to this email directly, view it on GitHubhttps://github.com/designly1/nextjs13-progress/issues/1#issuecomment-1866318534, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALHCNZDRTEOJ6VK3AYP7D3DYKQ62JAVCNFSM6AAAAABA5MS3D6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRWGMYTQNJTGQ. You are receiving this because you commented.Message ID: @.***>