Missing "are you sure?" alert when closing the window

[lindapaiste]

Related to #2793. Here is the explanation of the current problem.

Originally posted by @lindapaiste in #2590 (comment)

@raclim I realized that we lost the code which shows the alert when you try to close the entire window. 😭 Not sure if I should add that in here or make a new PR?

p5.js-web-editor/client/modules/IDE/pages/IDEView.jsx

Lines 94 to 95 in d26d062

	// window.onbeforeunload = this.handleUnsavedChanges;
	window.addEventListener('beforeunload', this.handleBeforeUnload);

This was something that @dewanshDT and I discussed and I wrote a code to handle it using react-router v6 syntax. But then we ended up staying on v5 and I guess it got lost in the shuffle. I don't think either of us wrote a function component v5 version.

Here is some of our discussion and the v6 code:

There's 3 different handlings:

1. when you close the window
2. when you click "new sketch"
3. when you navigate to a different page

These 3 situations are handled in 3 different places and we lost 1 of them. I totally understand why it's confusing because it took me a while to wrap my head around when I was working on the <WarnIfUnsavedChanges/>

refresh is equivalent to "close the window" in react-router terms because the whole app unmounts. so it's situation 1 that we are missing.

<WarnIfUnsavedChanges/> only handles #\3 and not #\1 because I was trying to keep the changes minimal when I wrote it. So #\1 was still being handled by the IDEView. Though now that we are refactoring the whole IDEView it makes sense to move that logic into the <WarnIfUnsavedChanges/>.

The logic that got lost is this:

window.addEventListener('beforeunload', this.handleBeforeUnload);

  handleBeforeUnload = (e) => {
    const confirmationMessage = this.props.t('Nav.WarningUnsavedChanges');
    if (this.props.ide.unsavedChanges) {
      (e || window.event).returnValue = confirmationMessage;
      return confirmationMessage;
    }
    return null;
  };

though now I'm not sure why the useBlocker can't handle this and I need to look at some react-router docs

You should work on something else and I will get this working. I think we basically need to move that code from the class component into a useBeforeUnload https://reactrouter.com/en/main/hooks/use-before-unload and we should put it in the <WarnIfUnsavedChanges/>

it's weird and annoying that we need both a useBlocker and a useBeforeUnload but it seems to be the case.

BTW I haven't been leaving a lot of comments on code that I write because this repo tends to not use comments but I find that kinda dumb tbh.

Like this seems like it needs a comment explaining what each hook is for.

ok I have it working. there's some dumb inconsistencies with browser handling of the beforeunload event and Chrome displays a standard browser message instead of our translated message but that was happening in the current production version too.

function WarnIfUnsavedChanges() {
  const hasUnsavedChanges = useSelector((state) => state.ide.unsavedChanges);
  const { t } = useTranslation();

  const currentLocation = useLocation();

  // blocker handles internal navigation between pages.
  const blocker = useBlocker(hasUnsavedChanges);

  useEffect(() => {
    if (blocker.state === 'blocked') {
      const nextLocation = blocker.location;
      if (
        isAuth(nextLocation.pathname) ||
        isAuth(currentLocation.pathname) ||
        isOverlay(nextLocation.pathname) ||
        isOverlay(currentLocation.pathname)
      ) {
        blocker.proceed();
      } else {
        const didConfirm = window.confirm(t('Nav.WarningUnsavedChanges'));
        if (didConfirm) {
          blocker.proceed();
        } else {
          blocker.reset();
        }
      }
    }
  }, [blocker, currentLocation.pathname, t, hasUnsavedChanges]);

  // beforeunload handles closing or refreshing the window.
  const handleUnload = useCallback(
    (e) => {
      if (hasUnsavedChanges) {
        // See: https://developer.mozilla.org/en-US/docs/Web/API/Window/beforeunload_event#browser_compatibility
        e.preventDefault();
        const confirmationMessage = t('Nav.WarningUnsavedChanges');
        e.returnValue = confirmationMessage;
        return confirmationMessage;
      }
      return null;
    },
    [t, hasUnsavedChanges]
  );

  useBeforeUnload(handleUnload);

  return null;
}

might make sense to move into its own file.

also it could be a hook.

if we want.

the only reason it's a component now is so it could be used in the class version of IDEView

[lindapaiste]

There is no equivalent of the useBeforeUnload hook in react-router v5 which we are using. We need to attach the handler directly to the window beforeunload event. We were doing that previously in the class-component version.