fix: use location.state to store background route to prevent unmount #4062
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| import { useEffect } from 'react'; | ||
| import { useLocation, useNavigate } from 'react-router-dom'; | ||
|
|
||
| import { RouteUrls } from '@shared/route-urls'; | ||
|
|
||
| import { useLocationState } from '@app/common/hooks/use-location-state'; | ||
|
|
||
| /* | ||
| when modals are opened in a new tab they lose the location.state.backgroundLocation | ||
| this hook sets the backgroundLocation to be RouteUrls.Home to improve UX | ||
| */ | ||
| export function useBackgroundLocationRedirect() { | ||
| const { pathname, state } = useLocation(); | ||
| const navigate = useNavigate(); | ||
| const backgroundLocation = useLocationState('backgroundLocation'); | ||
|
|
||
| useEffect(() => { | ||
| void (async () => { | ||
| switch (true) { | ||
| // FIXME ReceiveCollectibleOrdinal loses state?.btcAddressTaproot in a new tab | ||
| // this can be improved to try and fetch btcAddressTaproot | ||
| case pathname === RouteUrls.ReceiveCollectibleOrdinal && !state?.btcAddressTaproot: | ||
|
There was a problem hiding this comment. For If you open a new tab this state is lost and although the modal opens, the For now I just send users back to There was a problem hiding this comment. Not a big fan of using a This is better for readability imo. My thoughts map to the first comment of the HN post of a guy who wrote an article doing this (https://news.ycombinator.com/item?id=26777090) re favouring early return. Separately—while identical behaviour of modals when doing a fresh load of the app would be ideal—if we need to add a lot of logic to handle this, then I wouldn't consider it a dealbreaker if the under layer doesn't load correctly. Contextually, in case of sign out, it isn't so important. There was a problem hiding this comment. I like using If we are OK with having an empty screen behind all modals opened in a new tab then we can avoid the addition of this logic. It will look like this, with home tabs but no tab content:
For |
||
| return navigate(RouteUrls.Home); | ||
|
|
||
| case backgroundLocation === undefined: | ||
| return navigate(pathname, { | ||
| state: { backgroundLocation: { pathname: RouteUrls.Home } }, | ||
| }); | ||
| default: | ||
| return false; | ||
| } | ||
| })(); | ||
| }, [backgroundLocation, navigate, pathname, state]); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this hook so that:
backgroundLocationto be the home pageKapture.2023-07-28.at.14.28.19.mp4
Without doing this, an empty screen appears behind the modal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking on this further: couldn't each modal component handle their own
backgroundLocationlogic?Where the sign out modal component has a check in its render that, if there's no
backgroundLocation, it should re-router to having a default value of home?Could work with a similar hook API, but where all the logic is colocated to the modal itself, rather than one big catch-all.
Something like:
which internally handles the
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we can definitely do that. I just implemented a catch all for now as
Homeseems like the logical background. I can change it to accept a preferred default routeI can give it a try but I think once you open a new tab the
location.stategets wiped completely.