remix-run / react-router

Declarative routing for React
https://reactrouter.com
MIT License
53.09k stars 10.31k forks source link

[V6] [Feature] Getting `usePrompt` and `useBlocker` back in the router #8139

Closed callmeberzerker closed 1 year ago

callmeberzerker commented 3 years ago

I think in general most people won't be able to upgrade to v6 since in the latest beta usePrompt and useBlocker are removed.

Most apps rely on the usePrompt or Prompt to prevent navigation in case the user has unsaved changes (aka the form is dirty).

With this issue maybe we can have some feedback on why they (usePrompt, Prompt) were removed (they worked fine in the previous beta version v6.0.0-beta.6) and what makes them problematic, what's the outlook for getting them back in the router and potentially userland solutions to this problem.

chaance commented 2 years ago

As an FYI, there are more use cases for block than just "put up a dialog". I have an implementation of a wizard that will prevent the user from moving forward or backward when there is a condition that requires the user to fix something, such as waiting for an API to finish, dealing with API errors, etc.

Mind sharing a simplified repro? I'd love to take a look and see if we can work this into our examples.

slashwhatever commented 2 years ago

Then, when the user returns, repopulate the default values of all the form fields with the ones you saved from the last visit.

And how might that work in a multi user application where the default values saved no longer match with what's persisted in the DB?

A lot of this conversation seems to be centered around the idea that we're dealing with a single user. I would hope for most people that this is not the case and there are plenty of people using your app concurrently :)

slashwhatever commented 2 years ago

As an FYI, there are more use cases for block than just "put up a dialog". I have an implementation of a wizard that will prevent the user from moving forward or backward when there is a condition that requires the user to fix something, such as waiting for an API to finish, dealing with API errors, etc.

Couldn't you just hang the disabled state of your navigation buttons on the valid state of your form?

Also, as an aside, how are you finding creating a route based wizard? I've shied away from using routes in wizards previously because I don't want users to be able to bookmark, for example, page 4 of 5 when page 1 and 2 have required fields.

djswilly commented 2 years ago

At present the v5 migration guide states:

We will absolutely be working on adding this back in to v6 at some point in the near future, but not for our first stable release of 6.x.

This comment suggests that is no longer the case, is it worth updating the migration guide to reflect this?

mauro-ni commented 2 years ago

Hi, I agree with what @xr0master says, calling an "ugly built-in popup" in one line to block navigation is easier than implementing a draft state (and also works on multiple situations).

In my application I use useBlocker / usePrompt to avoid data loss in long forms implemented with react-hook-form library (I still have a few forms developed with formik that I have to migrate in the future).

I'm thinking about how to implement draft state management for long forms to avoid data loss. I would like to provide a reusable "piece of code" (one for rhf and one for formik). What do you think about the following workflow?

  1. The use work on a form and when he leaves the page I save a draft in session storage (record type, record id (if available), record updated at (if available), the state of the form)
  2. When the user returns to the form I check for a matching in the session storage and eventually show a popup to ask the user whether he wants to restore the state or delete the draft.

Am I missing anything?

Many thanks in advance.

Mauro

fzaninotto commented 2 years ago

Just a heads-up: the folks maintaining React Admin aren't updating their code because they think you will be reintroducing this method. Since it looks like you won't, I guess someone should let @fzaninotto know that a different approach is needed in order to stay compatible with the newest versions of React Router.

For your information, react-admin is removing the feature that was based on navigator.block (see https://github.com/marmelab/react-admin/pull/8272). We're sad about it.

verhaniamit commented 2 years ago

Do we know what level and how long we will have support for v5 going forward please? We have a situation where we would like to use the history.block feature but not sure how long v5 will be supported with respect to compatibility bugs/issues with React 18.

gius commented 2 years ago

https://github.com/TanStack/router seems like an interesting alternative (based on the same history library).

chaance commented 2 years ago

Do we know what level and how long we will have support for v5 going forward please?

We have no plans to stop supporting v5. If you have issues with React 18, please open an issue and we will address it there.

RobertGardner commented 2 years ago

It looks like we are going to be looking for a different router solution. This feature is critical for our application.

The problem with auto-saving on every change is that often fields in a form depend on each other and the entire form needs to be filled out before saving. If we just store the form in local storage, the user will think it's saved permanently and visible to other users, when it is not. Auto-saving when the user navigates away might work, but that also risks creating inconsistent data that could be surprising to the user -- sometimes they navigate away or refresh on purpose to cancel edits.

I can't think of a good solution for our data-heavy app that doesn't surprise the user.

avinoamsn commented 2 years ago

@RobertGardner our data-heavy app is currently using a version of the gist linked in https://github.com/remix-run/react-router/issues/8139#issuecomment-977790637 w/ react-router pinned to 6.2.2 (our implementation doesn't work on the latest release). Given that this is a workaround that relies on implementation specifics, it's not a long-term solution - but it works for us for the time being.

yarinsa commented 2 years ago

@avinoamsn Do you mind sharing what such an implmentation requires? Anything extra you did except the proposed solution? Did you managed to point why version higher then 6.2.2 doesn't work?

mgomez-tubs commented 2 years ago

The upgrading guide in https://reactrouter.com/en/main/upgrading/v5 should be updated to reflect the current situation of #8139 and #8035, which are very popular features in v5 that aren't and might not be supported anymore in the future.

The "few breaking changes" text is very misleading since this there are features that many users of this library expect to at least have a working workaround in react router v6. Also this thread should also be included in there, so users can see this discussion.

MarcusChrist commented 2 years ago

Working Prompt https://gist.github.com/MarksCode/64e438c82b0b2a1161e01c88ca0d0355

"react": "^18.2.0", "react-dom": "^18.2.0", "react-router-dom": "^6.4.2",

trb commented 2 years ago

I'm really confused by the unwillingness to support some method to block navigation for one reason: uploads.

How am I supposed to prevent users from losing progress on large uploads because they accidentally triggered a reload/navigation? People might upload gigabytes which can take hours, depending on their connection. I don't want them to lose progress because of an errand swipe on a trackpad or because they bumped into a button on their mouse.

What's your recommended way of handling large uploads? Not blocking navigation seems like a significant negative impact on user experience.

Do we know what level and how long we will have support for v5 going forward please?

We have no plans to stop supporting v5. If you have issues with React 18, please open an issue and we will address it there.

I don't mean to tease, but at the top of the thread you (as in, the organization) also said:

We will absolutely be working on adding this back in to v6 at some point in the near future, but not for our first stable release of 6.x.

So at this point my trust has been eroded a bit. I know I can just use another router, but that's hours of work just to address a - to me - fairly obvious use case (good UX for large uploads) and I find that frustrating.

Please at the very least make it obvious you don't and won't support blocking navigation.

conorluddy commented 2 years ago

I just landed here while looking for this feature too. In the app I'm working on I want it for the standard "if form dirty, prompt user before nav" use case. I have an ugly working workaround in place that I was hoping to not have to use, but I'll share it here in case anyone finds it helpful.

I'm using Formik for the form, but the app navigation is an ancestor of that, so the first step is to get the dirty flag from Formik and put it somewhere that the whole app can read it. In my case it's in a Recoil Atom, but you could use Context or whatever.

Step 2 was that I made a new useNavigation hook to "wrap" the react router one. Using this to replace existing useNavigate for the instances that are available/visible in the UI when on that form page.

Screenshot 2022-10-27 at 12 13 23

Step 3 was to do similar for a <Link /> in a Breadcrumb, using onClick to intercept, fire a prompt, and use event.preventDefault if the dialog is cancelled.

Screenshot 2022-10-27 at 12 14 48

This is ugly and incomplete, hence why I'm here looking for a better approach, but just sharing in case it might get someone out of a hole ❤️

Great router none the less, keep up the good work!

Edit: I've gone with this solution since the 6.4.3 release the other day broke the alternative "unstable" history.block solution. Just be careful if you end up implementing something like this that you'll need to ensure the global flag is unset when you leave the page or form. I'm doing that by falsifying it on the page unmount.

NerdCowboy commented 2 years ago

Instead of blocking navigation, another valid way to handle this situation could be to just save the state of the form to local/session storage in the browser as the form values change. Then, when the user returns, repopulate the default values of all the form fields with the ones you saved from the last visit. This experience should be less jarring than throwing up a prompt in front of the user while still giving you the resilience you're looking for.

Take it from a UX designer/dev:

Saving to local/session storage only solves the problem of losing entered data on a form (with a caveat for uploads). This is something that is necessary, but insufficient for good form UX.

Unsolved problems:

These might seem like trivial problems, but forgetting to hit submit can have major detrimental implications (ie losing money/customers). Larger files and/or people on slow connections will lose a large amount of time re-uploading. When you're under the gun, this also elevates to a critical problem.

Relying on internal implementation details was never what we advised. I'll take it a step further and advise that you should not rely on implementation details, and doing so means you accept any risk that your code will break in future updates.

This isn't really fair. We accepted the risk it would break, but we didn't accept the risk of reneging on adding back the feature to the API. It's a little disappointing to hear it phrased this way as it shifts responsibility onto us for a broken promise.

I understand the complexity of this problem and not wanting to solve all edge cases. However, I don't understand ripping out everything that could enable us to solve this problem on our own.

crates commented 2 years ago

So goes the story with React Router, which I've used since almost its very beginning. Constant changes to the API which are frequently incompatible with earlier versions for oft-questionable or unpopular reasons. Seriously, other than webpack, I have a hard time thinking of anyone who has burned me more often than React Router. I understand it's well-intentioned, but listen to your audience. You brilliant creators and contributors aren't the only one in the room. Why is it such a travesty to afford optional support for something so many people clearly need? These are just the ones in touch enough and vocal enough to be here contributing to the conversation; for each voice on this thread, there are a thousand more like us who aren't here to speak. You're messing with React Admin, one of my favorite and most-useful products. Is this all really worth it?

yarinsa commented 2 years ago

@NerdCowboy @crates @conorluddy @rmorse All of you are tagged cause you offered solution/showed disappointment of the library. I am assuming we can extend ReactRouter functionality to re-include the feature. At the beginning it wont be stable, but perhaps with the power of the community we will make it stable? WDYT ? Should we start a channel somewhere to initiate the project?

crates commented 2 years ago

@yarinsa - thanks for including me. I'm all for prospectively doing that, if the primary contributors and maintainers of the project are amenable. Perhaps the React Admin team might be willing to help as well. My time is extremely short at the moment (I'm in the middle of launching something massive and demanding, for which I'm the chief architect and project lead). But I'll try to help however I can. Whatever it takes. This is important functionality - the popularity of this thread makes that clear - and we shouldn't let an important capability get thrown out without a viable alternative.

With that said, I have very little context for the technical requirements in this situation. My take is, it worked in v5; it should be doable in v6. But it's entirely possible that I've missed some critical details as to why the RR team are fighting this so ardently. I'll freely admit that.

Let's brainstorm, if we can get more than just the two of us to commit and we're not fighting an uphill battle with the leaders of the project.

conorluddy commented 2 years ago

Definitely wasn't showing disappointment here anyways - I understand these things are complex and sometimes you need to make compromises for stability. Thanks for including me. Would love to help out!

TrejGun commented 2 years ago

Just fyi, it's still possible to use navigator.block via history package with v6.4.0, by replacing BrowserRouter with unstable_HistoryRouter, upgrade steps

  • add history package
  • use unstable_HistoryRouter, rather than BrowserRouter
import React from 'react';
import ReactDOM from 'react-dom/client';
import { unstable_HistoryRouter as HistoryRouter } from 'react-router-dom';
import { createBrowserHistory } from 'history';

import App from './App';

ReactDOM.createRoot(document.getElementById('root')).render(
  <React.StrictMode>
    <HistoryRouter history={createBrowserHistory({ window })}>
      <App />
    </HistoryRouter>
  </React.StrictMode>
);
  • add hook that use navigator.block, like useBlocker

https://stackblitz.com/edit/github-fqzmgn-ktacur-historyrouter

@westprophet checkout the example

This worked well until the minor release 6.4.3 now it shows an error

Screen Shot 2022-11-03 at 13 29 24

guys, honestly, FUCK YOU! You are not the only one in the room, some real-world applications rely on your code. No other NPM which I use breaks my app as often as you do. And yes I noticed unstable_ prefix. UNSTABLE_FUCK_YOU_AGAIN!

TrejGun commented 2 years ago

can I please have a hundred thumbs down? this will significantly amuse my ego :D

chaance commented 2 years ago

Friends, I think many of you have made some convincing arguments, so we're opening this back up for consideration. Thank you all who have [constructively and kindly] contributed to the conversation and made your case. We realize there are just some edge cases where our preferred approaches likely won't work, though this will need some love in the documentation to ensure folks know how to use these APIs responsibly.

Next step for us: we'll be drafting an RFC outlining these APIs in a 6.4 world. Chances are they might behave slightly differently compared to v5, as our intent is to fix some of the problems noted above. We'll want to capture all of those differences so you'll know what to expect when upgrading.

ryanflorence commented 2 years ago

I want folks on this issue to know that it’s impossible to do in a stable way.

This isn’t “react router won’t fix this” it’s “browsers don’t provide a way to reliably implement this”.

While we don’t like shipping apps or framework features that are fundamentally broken, the saturation of this feature in our user base is too big to have removed it.

So we’ll add it back, document where it’s irrecoverably broken, and let you do what you want 🥲

The rest of v6 and v6.4 and remix are too good for us to keep this roadblock in your way.

antoinm commented 2 years ago

@ryanflorence When you say that "it’s impossible to do in a stable way", could you please explain a bit further what it means? Do you consider the behaviour to block navigation implemented in v5 as stable ? I struggle to understand why this feature became difficult to implement in v6 because browsers don't provide a reliable way of doing it, but it works fine in v5 with the same browsers.

Thank you for your answers and all the efforts you guys are putting to get this feature back in v6!

ryanflorence commented 2 years ago

It’s not reliable in v5 either. Part of this work will be documenting how it’s broken so you can decide if it’s worth it.

The point of react router has always been “don’t break the back button” and this feature sometimes breaks the back button.

ianpaschal commented 2 years ago

The point of react router has always been “don’t break the back button” and this feature sometimes breaks the back button.

Sorry, sorry, I know you said more explanation is coming but maybe someone else could also answer... Isn't this, kind of the point? To give a big filled in form (for example) protection against being emptied out by an accidental click of the back button? I get the principle behind the sanctity of the browser but I also think it's important to acknowledge that sometimes OS's and browsers make UX decisions (like swiping the trackpad to go back, some key combination I kept accidentally hitting on an unfamiliar keyboard last week, etc.) that impact the usability of products we build. When that happens users don't get angry with the browser, they get angry that they just lost all their work and our site didn't save any their work in progress.

I guess what I'm trying to say is... if that's the philosophy of react-router then maybe this should be a separate library or something because blocking browser navigation which will cause my users to lose data is exactly what I'm aiming for (overridable with confirmation obviously).

Mario-Eis commented 2 years ago
  • If you absolutely need usePrompt or useBlocker, v5 is for you. It is stable and actively maintained.

But on the other hand:

It’s not reliable in v5 either. Part of this work will be documenting how it’s broken so you can decide if it’s worth it.

@ryanflorence Well, is v5 stable now, or is it impossible to do it in a stable way?!

What are those issues? Here is a thread with hundreds of active developers listening. Everyone with ideas and knowledge. Maybe explain those issues and hear what people throw at them.

I have customers asking for this blocking feature. And I don't know what to answer to our sunken costs caused by several update-revert-workaround hours. And relying on a promise from above, that the feature will be added again. Now there is a new statement in the room that basically is "Ok, we see that we have to add something back in. But it will behave different and it will be unstable". If there is no clear statement, I think the best advice would be to look for alternative router implementations like to one suggested here in the thread. And please don't get me wrong! I am really impressed by the work you do and respect every bit of it. People just can't answer to their stakeholders any more. The current state of things won't get us any further in a future proof manor. It also feels really bad to use the "old" version. The one that "has to" be kept alive because of all the users. Instead of the new shiny one that gets all the love and attention. That just doesn't feel very trustworthy any more.

chaance commented 2 years ago

What are those issues? Here is a thread with hundreds of active developers listening. Everyone with ideas and knowledge. Maybe explain those issues and hear what people throw at them.

As Ryan said, we are working on it. The nuances are edge-case-y and tricky to explain, but that's what we'll be documenting in the process of adding the feature back.

dietergeerts commented 2 years ago

What are those issues? Here is a thread with hundreds of active developers listening. Everyone with ideas and knowledge. Maybe explain those issues and hear what people throw at them.

As Ryan said, we are working on it. The nuances are edge-case-y and tricky to explain, but that's what we'll be documenting in the process of adding the feature back.

What really bugs me here is that there are routing packages out there that are doing these features for ages, without any issues. And yes, maybe they aren't perfect code wise, but what is more important, getting a code orgasm or have a feature that works? And if you now have issues, that might be because the way things are currently done aren't playing nice together? So as per suggestion, please share these issues, so the community can help.

trb commented 2 years ago

For me, I don't think I'd expect (or maybe event want) the router to handle prompting, or hooking into onbeforeleave. I feel like that's a concern that something else should handle, maybe a bespoke hook.

But to implement such a hook I need to be able to prevent the router from processing back/forward events from the browser. I have no way to disable those otherwise, as onpopstate doesn't support stopPropagation. For everything else I can add event handlers that prevent propagation, although it'd be nice not to need to do that for elements managed by the router (e.g. \<Link>s).

But for onpopstate there's no escape hatch.

All I need is for react-router to provide a way to enable/disable processing navigation events, like history.block()/.unblock(). That's enough of an escape hatch to write a hook/component to handle fully blocking navigation. I think usePrompt could be implement as a contribution-level package.

crates commented 2 years ago

@ryanflorence - I (occasionally) have the ear of the head of the Webkit team at Apple, responsible for all iOS-based browsing. I might be able to get in touch with some of the other folks who are in charge of supported browser features with other major providers as well. Can you help me understand what additional browser support would be valuable to making this easier to achieve? If I can grok your need, I will gladly pass it along to other parties and see if we can make progress on that front.

Full disclosure - changes to browser standards usually take at least a few months, if not a year to attain... but they ARE possible with the right communication and parties involved. I'm happy to get that moving if it helps everyone here.

vocko commented 1 year ago

I want folks on this issue to know that it’s impossible to do in a stable way.

This isn’t “react router won’t fix this” it’s “browsers don’t provide a way to reliably implement this”.

While we don’t like shipping apps or framework features that are fundamentally broken, the saturation of this feature in our user base is too big to have removed it.

So we’ll add it back, document where it’s irrecoverably broken, and let you do what you want 🥲

Thank you Ryan, I think this is the best possible approach - tell me the risks and I will decide whether they are worth it or not. Particularly because this is not about a new feature but something that was in the system previously and many apps rely on it.

I can think of at least 3 scenarios where blocking the state is either the only option or the by far easier one: 1) File upload in progress (already uploaded data will be lost, you have to start over) 2) Form data are sensitive (you cannot store them) 3) Form is stateful and data restoration is very complicated (requires BE calls etc)

Beside that, in our app we have dozens, if not hundreds, of forms of which many have some unique behavior. Either side effects or specific validation rules and rewriting all of them to temporary store the state is a huge chunk of work. And I think this is the case of many others - we will rather take the risk that back button functionality will be broken in some cases than invest hundreds of hours to re-code all our existing components.

Thank you again work your work and communication guys, very appreciated.

MattiasMartens commented 1 year ago

Router navigation is a use case that affects every SPA framework. As evidenced by the discussion here, blocking prompts are a well-established component of that use case.

React’s other router frameworks did not take the step of removing support for the use case. Vue.js router frameworks did not take this step. I can't speak to Angular or Svelte. My point is, to my knowledge there's been no mass exodus away from router-blocking prompts in SPA frameworks, and no recent W3C, GDPR, or SEO-related announcement that might have triggered one.

I know this question has been asked many times already but I will repeat: there is a great urge to know what react-router knows that no-one else seems to. What's the missing information?

If there is none, why was this decision made?

If there is, does react-router know that no-one else has this information?

There is an ongoing informal process where framework authors and framework consumers converge on standards as to what features consumers can reasonably expect. The ability to halt or delay router navigation does seem to be one of those features——at least, it is needed, and it’s a feature that a consumer can’t easily replace if it’s not provided (given that, as others have said very well, local storage of form state is an inadequate alternative, both functionally and for UX).

If existing browser standards are not sufficient to support navigation blocking within an SPA, then a partial implementation at least points out that browser insufficiency and makes it legible to developers and users. Yanking the insufficient implementation out sends the opposite message: that this problem is not one to be solved, but avoided.

I hope it’s clear at the end of all this that, in fact, the problem does need to be solved, even if browsers do not yet provide the tools to solve it in all cases.

chaance commented 1 year ago

Just to reiterate, we are working on this. No need to continue making the case for something we're already doing!

I'm going to lock the conversation to contributors for now since there are a ton of folks subscribed getting pinged here, but we'll post updates as things progress.

ryanflorence commented 1 year ago

Sorry, sorry, I know you said more explanation is coming but maybe someone else could also answer... Isn't this, kind of the point? To give a big filled in form (for example) protection against being emptied out by an accidental click of the back button?

Yes that's the point, no it's not what breaks the back button. The history stack can get screwed up after a block and then a valid "back button click" goes to the wrong place. The URLs also don't always reflect the state of the app (a refresh would show a different UI).

ryanflorence commented 1 year ago

@Mario-Eis

"Ok, we see that we have to add something back in. But it will behave different and it will be unstable"

It will be more stable in v6, but still have edge cases you can't do anything about. It is unstable in v5 in your apps right now (and all other client side routers with this feature).

ryanflorence commented 1 year ago

@trb

But for onpopstate there's no escape hatch.

All I need is for react-router to provide a way to enable/disable processing navigation events

All we have access to is onpopstate too 😅

ryanflorence commented 1 year ago

@crates

Can you help me understand what additional browser support would be valuable to making this easier to achieve?

The new Navigation API proposal has what we need. We spoke with the google team behind it a couple years ago about it. So implement that and we're good :)

https://github.com/WICG/navigation-api/issues/32#issuecomment-790936485

chaance commented 1 year ago

Hi friends! Unlocking this conversation again as we start to work on the new implementation. I'd like to get as much feedback as possible so that we can avoid as many problems as we can along the way.

Does your app use the getUserConfirmation prop to customize the <Prompt> experience?

If so, we'd really love to see what you're doing here. If you can share your use case (minimal code demos would be super useful!), that would help guide us to make the right decisions here.

N1kto commented 1 year ago

@chaance it would be great to have at least what previous implementation provided. For our use cases in several apps it was very well enough. The core feature of Prompt was blocking the user navigation and possible data loss as a result. All the rest is extra (like customizing the confirmation dialog) which is nice to have.

Never happened to need getUserConfirmation prop. When customization was needed we used history.block to build custom prompt around it.

P.S. for those who ended up here while searching for possible workaround of blocking user navigation in react-router v6, here is a gist which works for me. I wanted to post it earlier but comments used to be disabled. Looking forward to "built-in" blocking api in react-router 6 to be able to get rid of that hacky workaround though.

hxlniada commented 1 year ago

hello, when will them be back?

lexanth commented 1 year ago

@chaance In v5 we were using a callback message to use a custom modal dialog. In v6 (6.2.1), we're using navigator.block to use a custom modal dialog like this: https://stackblitz.com/edit/github-qjq8bo?file=src/useRouteLeavingGuard.ts although it feels quite hacky.

For us, while window.confirm is convenient as a default, custom modal dialog is what the designers/product people want, so we want to be able to provide a react component (either with state like this, or a render prop).

yched commented 1 year ago

@chaance

Does your app use the getUserConfirmation prop to customize the experience?

If so, we'd really love to see what you're doing here. If you can share your use case (minimal code demos would be super useful!), that would help guide us to make the right decisions here.

We migrated our app to v6 and had to resort to one of the useBlocker() implementations (based on UNSAFE_NavigationContext) offered at the top of the thread

Back in v5, we used getUserConfirmation() on the BrowserRouter yes.

In order to be consistent with the rest of our UI (notably with other similar but not router-related confirmation dialogs), we don't rely on the native window.confirm(), we use material-ui confirmations modals, whose display is controlled by some redux state (app from 2016, still largely redux-based...).

Thus our getUserConfirmation() calls an async callback, which is actually as async redux thunk that resolves to true / false when the user confirms. Additionally, since the getUserConfirmation() callback only lets us receive a message string, while our confirm modals show a title + a more detailed explanation below, we set Prompt's message prop as a pipe-delimited string ('bla bla title|ladida description"), and split it before sending it to our confirm() thunk :

// redux thunk action 
function confirm(messageData) {
  return (dispatch) => new Promise(resolve => {
    dispatch(showConfirm({
      messageData,
      onConfirm: () => resolve(true),
      onCancel: () => resolve(false),
    }));
  });
}

function AppRouter(props) {
  const dispatch = useDispatch(); // redux dispatch
  const getUserConfirmation = async (string, callback) => {
    const [title, description] = string.split('|', 2);
    callback(await dispatch(confirm({ title, description })));
  };
  return <BrowserRouter getUserConfirmation={getUserConfirmation} {...props} />;
}

We then have an (always mounted) ConfirmModal component that reads { messageData, onConfirm, onCancel } from redux store and, if set, shows the corresponding material-ui dialog, plugging onConfirm and onCancel to the corresponding buttons.

mikeymop commented 1 year ago

We do something similar to yched in our application, and also use the implementation towards the top of this thread. We use getUserConfirmation however we instead default to true and show a toast message. In our useBlock call we store the user's form data in case we need it later.

If the user interacts with the toast message we fire the previously blocked navigation state to return the user to the form and prefill the stored formdata.

We accomplish this by leaving an always mounted global snackbar that is invisible until needed. This enables the snackbar to remain on screen even across route transitions.

This effectively recreates the Material Design guidelines for snackbars and destructive actions where it is stated that a user should be able to perform a destructive action without confirmation but the application should offer an option to 'undo' the destructive action.

To put it shortly, we dont strictly depend on 'blocking' a route transition. But instead rely on being able to perform arbitrary actions based on a route transition when a user is in an unsubmitted form.

awesomeunleashed commented 1 year ago

@chaance we don't use getUserConfirmation, but instead rely on the boolean return option from Prompt.message, specifically the ability to return false to block without a message. Here's a slightly simplified version:

const [blockedLocation, setBlockedLocation] = useState(null);
const history = useHistory();

return (
    <>
        <Prompt
            message={(location) => {
                if (blockedLocation) {
                    setBlockedLocation(null);
                    return true;
                }
                setBlockedLocation(location);
                return false;
            }}
            when={when}
        />
        <UnsavedModal // our "you have unsaved changes" modal
            onClose={() => setBlockedLocation(null)} // cancel and don't go anywhere
            onContinue={() => history.push(blockedLocation)} // discard your changes and go to what you clicked on
            show={!!blockedLocation}
        />
    </>
);

It feels slightly hacky, but it has worked well for us.

kwatkins-ld commented 1 year ago

@chaance We're not using the Prompt component, but we have a custom hook that wraps history.block() (imported from the history package), and then a component that layers in analytics for confirming/canceling the navigation, allow-listing of certain nested route transitions, etc.

Are y'all planning an equivalent low-level API like history.block?

chaance commented 1 year ago

Are y'all planning an equivalent low-level API like history.block?

We don't expose history at all anymore, but we are re-implementing history.block so you can provide your own history instance to the router if you really need it.

We will provide a lower-level hook for blocking, but it will work a bit differently. That will probably come after Prompt and we'll make sure to document that carefully.

CreativeTechGuy commented 1 year ago

@chaance Our use-case uses <Prompt> but we cannot rely on the when boolean prop so we had to use the undocumented behavior of the message prop function return value. So we (ab)used the message function to check if the navigation should be blocked or not.

The use-case where this is necessary is the following:

That approach is a bit hacky. So to sum it up, what we need is a way to dynamically determine if the navigation should be prevented at the moment of navigation so we can determine if it should be blocked right there. We cannot always determine it ahead of time. So if the old when prop accepted a function that returned true/false, that'd be perfect!