Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Controlled Interceptor inserting its interceptorFunction into the address bar on navigate #47

Closed
bvalyou opened this issue Apr 26, 2019 · 6 comments

Comments

2 participants
@bvalyou
Copy link

commented Apr 26, 2019

I'm trying to apply useControlledInterceptor to manage an unmount navigation on a generic component.

const [className, setClassName] = useState(classes.incoming);
const [nextPath, confirmNavigation] = useControlledInterceptor();

useEffect(() => {
	if (!nextPath) {
		return;
	}

	setClassName(classes.out);
	setTimeout(confirmNavigation, 200);
}, [nextPath]);

Outside of this component I have <A href="/">...</A>, and when I click this link (or perform any other behavior in the app that causes navigation) I get this url in my address bar http://localhost:3000/function%20(nextPath,%20currentPath)%20%7B%20%20%20%20%20%20setInterceptedPath(nextPath);%20%20%20%20%20%20return%20currentPath;%20%20%20%20%7D and my 404 page displays.

I assume that this is because the interceptors don't link in with the router context so it only works with navigation caused from within the same component, but I haven't dug into it too far yet.

@Paratron

This comment has been minimized.

Copy link
Owner

commented Apr 26, 2019

Do you happen to have a codesandbox or something to demonstrate the behavior? Otherwise I would need to recreate the bug, first to find the cause.

@Paratron

This comment has been minimized.

Copy link
Owner

commented Apr 26, 2019

Nevermind, I was able to recreate it. I will investigate.

@Paratron Paratron added the bug label Apr 26, 2019

@Paratron Paratron added this to In progress in Dev Apr 26, 2019

Paratron added a commit that referenced this issue Apr 26, 2019

@Paratron Paratron moved this from In progress to Done (to be released) in Dev Apr 26, 2019

@Paratron

This comment has been minimized.

Copy link
Owner

commented Apr 26, 2019

Fixed with the released version 1.1.7

@Paratron Paratron closed this Apr 26, 2019

@bvalyou

This comment has been minimized.

Copy link
Author

commented Apr 26, 2019

I hadn't had a chance to look at this again until just now, but thanks for the quick fix! I pulled down 1.1.7 and confirmed that it is fixed.

This functionality makes animated transitions between pages almost effortless! This is the full code (besides the CSS) to animate in and out.

const PageRoot = (props) => {
	const {
		title, children, classes, theme,
	} = props;
	const [className, setClassName] = useState(classes.incoming);
	const [nextPath, confirmNavigation] = useControlledInterceptor();

	useEffect(() => {
		if (!nextPath) {
			return;
		}

		setClassName(classes.out);
		setTimeout(confirmNavigation, theme.transitions.duration.short);
	}, [nextPath, classes, confirmNavigation, theme]);

	useEffect(() => {
		setTimeout(() => setClassName(classNames(classes.incoming, classes.in), 1));
	}, []);

	return (
		<main className={classes.root}>
			<Header Icon={props.headerIcon}>{props.headerContent || title}</Header>

			<div className={classNames(classes.mainContent, className)}>
				{children}
			</div>
		</main>
	);
};

This is a huge win for this library, and the good work here is much appreciated! Feel free to lift that code and add to the docs, or if there's a spot you'd like to see them I can put in a PR as well.

@Paratron

This comment has been minimized.

Copy link
Owner

commented Apr 26, 2019

Thats a very creative use of the controlled interceptor :D
You could even make that functionality a new hook like useTransition() and pass the in- and out-classnames as well as the duration as arguments!

@bvalyou

This comment has been minimized.

Copy link
Author

commented Apr 27, 2019

That's a great idea! I've done that, just passed in my classes hash and kept the names the same. Makes the PageRoot component much simpler and I can reuse the logic in other places and test in isolation.

One note for anyone who wants to try this - the one issue I ran into is on pages that have parameters and can be loaded twice consecutively, it transitions away, but then the transition back in doesn't happen because the transition in effect only runs on mount. My fix was to add a key of window.location.pathname to the transitioning component where it's rendered by components with parameters. I also tested adding window.location.pathname to the effect that causes it to transition in, but that causes the animation to run in reverse (from out state back to in state rather than from initial to in).

Thanks again for your help @Paratron. I did notice one thing that might wind up being a bug for someone later - nextPath doesn't reset after calling confirmNavigation which means you can't explicitly run logic post-confirm in hook world (besides tracking confirm in a separate state field in the consumer). Changing that might require some extra code to gate intercepting the empty nextPath so not sure if you'd want that or not.

🥂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.