Skip to content
This repository has been archived by the owner on Nov 15, 2022. It is now read-only.

Confirm before navigating away from unsaved data #10

Closed
michaeljwilliams opened this issue Mar 25, 2019 · 14 comments
Closed

Confirm before navigating away from unsaved data #10

michaeljwilliams opened this issue Mar 25, 2019 · 14 comments
Labels
enhancement New feature or request
Projects

Comments

@michaeljwilliams
Copy link

michaeljwilliams commented Mar 25, 2019

Just finishing integrating hookrouter into my current project. I like it a lot so far! Very easy to implement and reason around.

On a certain page my user is filling out a form, and I want to confirm before they can leave, to prevent loss of unsaved data.

I think it would be better if this was baked into the package somehow. I'd want something simple where you could set it for a whole page and it applies to children and parents. I also would prefer to be able to pass any function, in case window.confirm() isn't good enough.

@Paratron
Copy link
Owner

Whats wrong with this approach?

if(confirm('Are you sure?')){
    navigate('/somewhereElse');
}

@Paratron
Copy link
Owner

Ah, you mean that you also want to prevent navigation by clicking on a link. I will make up my mind about that.

@michaeljwilliams
Copy link
Author

michaeljwilliams commented Mar 25, 2019

To use that, none of the links on the page could be <A />s. They'd have to be regular elements with an onClick={your code}. That would be really tedious to do if I had a bunch of links on the page. And it would be impossible for links in another component.

I'd be happy to contribute towards a solution, but I haven't thought of one yet.

@Paratron Paratron added the enhancement New feature or request label Mar 25, 2019
@Paratron
Copy link
Owner

I made a concept about how this could be implemented. This would also enable us to do some kind of conditional URL rewriting... Not sure about it, tough - I need to let it sink in.

import {useInterceptor} from 'hookrouter';
// Random form stuff
import {saveData} from 'dataStuff';
import {MyForm} from '../forms';

const myInterceptor = (nextPath, currentPath) => {
	if(confirm('Do you really want to leave?')){
		return nextPath;
	}
	return currentPath;
};

const GuardedForm = () => {
	const stopInterception = useInterceptor(myInterceptor);

	const handleSubmit = (data) => {
		saveData(data);
		stopInterception();
		navigate('/somewhereElse');
	}

	return (
		<MyForm onSubmit={handleSubmit} />
	);
}

A interceptor function is being called after a navigation intention has been made through either clicking an <A> tag or calling navigate().

The next path and the current path is passed to the interceptor and it may decide about the path to return. Whatever path the interceptor returns gets the new path to navigate to. If the interceptor returns the same path as the current path, nothing happens.

This would also give the possibility to "lock" just parts of the URL. For example you want to split up an input process about several sub-parts of the url but prevent the user from navigating away from the process as a whole, you could do it with an interceptor.

Example: Allow navigation between /register/general, /register/photo or /register/details but not to /otherPage.

@danielkcz
Copy link

danielkcz commented Mar 26, 2019

I don't think this will suffice. Navigation is not always explicit through the link, but the user can simply hit back button in the browser and this solution would not prevent that. Also sometimes it's useful to warn about unsaved data before closing a browser window, sometimes it's even being closed by a mistake.

That's why I was suggesting integration with history package. Why reinvent the wheel?

@Paratron
Copy link
Owner

I think I was not clear enough - the interceptor gets called on a navigation intent. Navigation intents can be triggered through link clicks, calls to navigate(), clicking back/forward in the browser.

Hookrouter is right now about 1.8kb in size. The history package alone would add ~3kb - just for one function that I can implement by adding a couple of lines of code. This does not justify installing such a dependency. Those need to outweigh the drawbacks of adding dependencies by far.

@danielkcz
Copy link

Ok, fair enough. It's kinda nice thinking out of the box indeed :)

One more thing with useInterceptor, would be really lovely if we could actually render React components when the intercept happens. The confirm can suffice for some simple stuff, but some apps want to use a custom modal.

@Paratron
Copy link
Owner

Paratron commented Mar 26, 2019

Well, by moving the intercept function into the react component function, it will be able to access state properties and setters. Then, you could actually tell the component that and which route has been intercepted and react on it:

import React from 'react';
import {useInterceptor} from 'hookrouter';
// Random form stuff
import {saveData} from 'dataStuff';
import {ConfirmPopup} from '../popups'
import {MyForm} from '../forms';

const GuardedForm = () => {
	const [interceptedPath, setInterceptedPath] = React.useState(null);
	// Will intercept any new path and write the intercepted path into the component state.
	const myInterceptor = React.useCallback(
		() => (nextPath, currentPath) => {
			setInterceptedPath(nextPath);
			return currentPath;
		},
		[setInterceptedPath]
	);
	const stopInterception = useInterceptor(myInterceptor);

	// Will stop the interceptor and perform the previously stored navigation intent.
	const confirmedNavigation = () => {
		stopInterception();
		navigate(interceptedPath);
	}

	const handleSubmit = (data) => {
		saveData(data);
		stopInterception();
		navigate('/somewhereElse');
	}

	return (
		<div>
			{ interceptedPath && <ConfirmPopup onConfirm={confirmedNavigation} /> }
			<MyForm onSubmit={handleSubmit} />
		</div>
	);
}

@Paratron
Copy link
Owner

This is too complicated. I think I will simply add another hook next to useInterceptor() - namely useControlledInterceptor() which does the above automatically but is much simpler to use:

const SimpleGuardedForm = () => {
	const [interceptedRoute, setInterceptedRoute] = React.useState(null);
	const [stopInterception, confirmNavigation] = useControlledInterceptor(setInterceptedRoute);

	const handleSubmit = (data) => {
		saveData(data);
		stopInterception();
		navigate('/somewhereElse');
	}

	return (
		<div>
			{ interceptedRoute && <ConfirmPopup onConfirm={confirmNavigation} /> }
			<MyForm onSubmit={handleSubmit} />
		</div>
	);
}

@danielkcz
Copy link

That looks great, definitely much easier than with react-router for sure :) Once this and #11 is resolved, I am seriously considering the switch. Great job man!

@michaeljwilliams
Copy link
Author

One more thing with useInterceptor, would be really lovely if we could actually render React components when the intercept happens. The confirm can suffice for some simple stuff, but some apps want to use a custom modal.

That’s why I was suggesting being able to pass a callback function when intercept is called.

@michaeljwilliams
Copy link
Author

michaeljwilliams commented Mar 27, 2019

I'm getting an error while trying to use useInterceptor. I cloned the repo and ran prepublishOnly and imported useInterceptor in my component. I copied the interceptor code from your test file. Now when my app compiles and I go to that route, I get the error below. Is this a bug or am I doing something wrong?

image

@danielkcz
Copy link

@michaeljwilliams I highly recommend https://github.com/whitecolor/yalc tool for installing unreleased packages. I did it already and works just fine for me. Haven't tried useInterceptor yet, but seeing it's exported correctly, it's more likely wrong on your side.

Also, note that with CRA it won't be able to load newly added (or updated) library from node_modules. You have to restart the process.

@michaeljwilliams
Copy link
Author

michaeljwilliams commented Mar 27, 2019

@FredyC I got it working using yalc; thanks for the advice. I used the following to make the app behave as expected (warns whenever trying to navigate away, if you confirm then it proceeds). I like this!

const stopInterception = useInterceptor((currentPath, nextPath) => {
    if(window.confirm("Are you sure you want to leave without saving?")) {
        stopInterception();
        navigate(nextPath);
    }
})

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
Version 2.0
  
Done (to be released)
Development

No branches or pull requests

3 participants