Simplify web push UX and updates

- Use a single endpoint - Use a declarative web push sync hook. This thus handles all edge cases that had to be manually handled before: logout, login, account sync, etc. - Simplify UX: browser notifications are always enabled (unless denied), web push toggle only shows up if permissions are already granted.
2023-06-02 13:22:54 +02:00 · 2023-06-02 13:22:54 +02:00 · 47ad024ec7
commit 47ad024ec7
parent 4944e3ae4b
20 changed files with 294 additions and 427 deletions
--- a/web/src/components/Account.jsx
+++ b/web/src/components/Account.jsx
@ -1078,8 +1078,6 @@ const DeleteAccountDialog = (props) => {

  const handleSubmit = async () => {
    try {
-      await subscriptionManager.unsubscribeAllWebPush();
-
      await accountApi.delete(password);
      await getDb().delete();
      console.debug(`[Account] Account deleted`);
--- a/web/src/components/ActionBar.jsx
+++ b/web/src/components/ActionBar.jsx
@ -120,8 +120,6 @@ const ProfileIcon = () => {

  const handleLogout = async () => {
    try {
-      await subscriptionManager.unsubscribeAllWebPush();
-
      await accountApi.logout();
      await getDb().delete();
    } finally {
--- a/web/src/components/Navigation.jsx
+++ b/web/src/components/Navigation.jsx
@ -108,27 +108,34 @@ const NavList = (props) => {
  const isPaid = account?.billing?.subscription;
  const showUpgradeBanner = config.enable_payments && !isAdmin && !isPaid;
  const showSubscriptionsList = props.subscriptions?.length > 0;
-  const showNotificationPermissionDenied = notifier.denied();
+  const [showNotificationPermissionRequired, setShowNotificationPermissionRequired] = useState(notifier.notRequested());
+  const [showNotificationPermissionDenied, setShowNotificationPermissionDenied] = useState(notifier.denied());
  const showNotificationIOSInstallRequired = notifier.iosSupportedButInstallRequired();
  const showNotificationBrowserNotSupportedBox = !showNotificationIOSInstallRequired && !notifier.browserSupported();
  const showNotificationContextNotSupportedBox = notifier.browserSupported() && !notifier.contextSupported(); // Only show if notifications are generally supported in the browser

-  const navListPadding =
+  const refreshPermissions = () => {
+    setShowNotificationPermissionRequired(notifier.notRequested());
+    setShowNotificationPermissionDenied(notifier.denied());
+  };
+
+  const alertVisible =
+    showNotificationPermissionRequired ||
    showNotificationPermissionDenied ||
    showNotificationIOSInstallRequired ||
    showNotificationBrowserNotSupportedBox ||
-    showNotificationContextNotSupportedBox
-      ? "0"
-      : "";
+    showNotificationContextNotSupportedBox;

  return (
    <>
      <Toolbar sx={{ display: { xs: "none", sm: "block" } }} />
-      <List component="nav" sx={{ paddingTop: navListPadding }}>
+      <List component="nav" sx={{ paddingTop: alertVisible ? "0" : "" }}>
+        {showNotificationPermissionRequired && <NotificationPermissionRequired refreshPermissions={refreshPermissions} />}
        {showNotificationPermissionDenied && <NotificationPermissionDeniedAlert />}
        {showNotificationBrowserNotSupportedBox && <NotificationBrowserNotSupportedAlert />}
        {showNotificationContextNotSupportedBox && <NotificationContextNotSupportedAlert />}
        {showNotificationIOSInstallRequired && <NotificationIOSInstallRequiredAlert />}
+        {alertVisible && <Divider />}
        {!showSubscriptionsList && (
          <ListItemButton onClick={() => navigate(routes.app)} selected={location.pathname === config.app_root}>
            <ListItemIcon>
@ -346,16 +353,36 @@ const SubscriptionItem = (props) => {
  );
 };

+const NotificationPermissionRequired = ({ refreshPermissions }) => {
+  const { t } = useTranslation();
+  return (
+    <Alert severity="info" sx={{ paddingTop: 2 }}>
+      <AlertTitle>{t("alert_notification_permission_required_title")}</AlertTitle>
+      <Typography gutterBottom align="left">
+        {/* component=Button is not an anchor, false positive */}
+        {/* eslint-disable-next-line jsx-a11y/anchor-is-valid */}
+        <Link
+          component="button"
+          style={{ textAlign: "left" }}
+          onClick={async () => {
+            await notifier.maybeRequestPermission();
+            refreshPermissions();
+          }}
+        >
+          {t("alert_notification_permission_required_description")}
+        </Link>
+      </Typography>
+    </Alert>
+  );
+};
+
 const NotificationPermissionDeniedAlert = () => {
  const { t } = useTranslation();
  return (
-    <>
-      <Alert severity="warning" sx={{ paddingTop: 2 }}>
-        <AlertTitle>{t("alert_notification_permission_denied_title")}</AlertTitle>
-        <Typography gutterBottom>{t("alert_notification_permission_denied_description")}</Typography>
-      </Alert>
-      <Divider />
-    </>
+    <Alert severity="warning" sx={{ paddingTop: 2 }}>
+      <AlertTitle>{t("alert_notification_permission_denied_title")}</AlertTitle>
+      <Typography gutterBottom>{t("alert_notification_permission_denied_description")}</Typography>
+    </Alert>
  );
 };

@ -363,7 +390,7 @@ const NotificationIOSInstallRequiredAlert = () => {
  const { t } = useTranslation();
  return (
    <>
-      <Alert severity="warning" sx={{ paddingTop: 2 }}>
+      <Alert severity="info" sx={{ paddingTop: 2 }}>
        <AlertTitle>{t("alert_notification_ios_install_required_title")}</AlertTitle>
        <Typography gutterBottom>{t("alert_notification_ios_install_required_description")}</Typography>
      </Alert>
@ -375,33 +402,27 @@ const NotificationIOSInstallRequiredAlert = () => {
 const NotificationBrowserNotSupportedAlert = () => {
  const { t } = useTranslation();
  return (
-    <>
-      <Alert severity="warning" sx={{ paddingTop: 2 }}>
-        <AlertTitle>{t("alert_not_supported_title")}</AlertTitle>
-        <Typography gutterBottom>{t("alert_not_supported_description")}</Typography>
-      </Alert>
-      <Divider />
-    </>
+    <Alert severity="warning" sx={{ paddingTop: 2 }}>
+      <AlertTitle>{t("alert_not_supported_title")}</AlertTitle>
+      <Typography gutterBottom>{t("alert_not_supported_description")}</Typography>
+    </Alert>
  );
 };

 const NotificationContextNotSupportedAlert = () => {
  const { t } = useTranslation();
  return (
-    <>
-      <Alert severity="warning" sx={{ paddingTop: 2 }}>
-        <AlertTitle>{t("alert_not_supported_title")}</AlertTitle>
-        <Typography gutterBottom>
-          <Trans
-            i18nKey="alert_not_supported_context_description"
-            components={{
-              mdnLink: <Link href="https://developer.mozilla.org/en-US/docs/Web/API/notification" target="_blank" rel="noopener" />,
-            }}
-          />
-        </Typography>
-      </Alert>
-      <Divider />
-    </>
+    <Alert severity="warning" sx={{ paddingTop: 2 }}>
+      <AlertTitle>{t("alert_not_supported_title")}</AlertTitle>
+      <Typography gutterBottom>
+        <Trans
+          i18nKey="alert_not_supported_context_description"
+          components={{
+            mdnLink: <Link href="https://developer.mozilla.org/en-US/docs/Web/API/notification" target="_blank" rel="noopener" />,
+          }}
+        />
+      </Typography>
+    </Alert>
  );
 };

--- a/web/src/components/Preferences.jsx
+++ b/web/src/components/Preferences.jsx
@ -86,7 +86,7 @@ const Notifications = () => {
        <Sound />
        <MinPriority />
        <DeleteAfter />
-        {notifier.pushSupported() && <WebPushDefaultEnabled />}
+        {notifier.pushPossible() && <WebPushDefaultEnabled />}
      </PrefGroup>
    </Card>
  );
--- a/web/src/components/SubscribeDialog.jsx
+++ b/web/src/components/SubscribeDialog.jsx
@ -12,16 +12,14 @@ import {
  FormGroup,
  useMediaQuery,
  Switch,
-  Stack,
 } from "@mui/material";
 import { useTranslation } from "react-i18next";
-import { Warning } from "@mui/icons-material";
 import { useLiveQuery } from "dexie-react-hooks";
 import theme from "./theme";
 import api from "../app/Api";
 import { randomAlphanumericString, topicUrl, validTopic, validUrl } from "../app/utils";
 import userManager from "../app/UserManager";
-import subscriptionManager, { NotificationType } from "../app/SubscriptionManager";
+import subscriptionManager from "../app/SubscriptionManager";
 import poller from "../app/Poller";
 import DialogFooter from "./DialogFooter";
 import session from "../app/Session";
@ -59,16 +57,16 @@ const SubscribeDialog = (props) => {

  const webPushDefaultEnabled = useLiveQuery(async () => prefs.webPushDefaultEnabled());

-  const handleSuccess = async (notificationType) => {
+  const handleSuccess = async (webPushEnabled) => {
    console.log(`[SubscribeDialog] Subscribing to topic ${topic}`);
    const actualBaseUrl = baseUrl || config.base_url;
    const subscription = await subscribeTopic(actualBaseUrl, topic, {
-      notificationType,
+      webPushEnabled,
    });
    poller.pollInBackground(subscription); // Dangle!

    // if the user hasn't changed the default web push setting yet, set it to enabled
-    if (notificationType === "background" && webPushDefaultEnabled === "initial") {
+    if (webPushEnabled && webPushDefaultEnabled === "initial") {
      await prefs.setWebPushDefaultEnabled(true);
    }

@ -100,23 +98,6 @@ const SubscribeDialog = (props) => {
  );
 };

-const browserNotificationsSupported = notifier.supported();
-const pushNotificationsSupported = notifier.pushSupported();
-const iosInstallRequired = notifier.iosSupportedButInstallRequired();
-const pushPossible = pushNotificationsSupported && iosInstallRequired;
-
-const getNotificationTypeFromToggles = (browserNotificationsEnabled, backgroundNotificationsEnabled) => {
-  if (backgroundNotificationsEnabled) {
-    return NotificationType.BACKGROUND;
-  }
-
-  if (browserNotificationsEnabled) {
-    return NotificationType.BROWSER;
-  }
-
-  return NotificationType.SOUND;
-};
-
 const SubscribePage = (props) => {
  const { t } = useTranslation();
  const { account } = useContext(AccountContext);
@ -134,27 +115,7 @@ const SubscribePage = (props) => {
  const reserveTopicEnabled =
    session.exists() && (account?.role === Role.ADMIN || (account?.role === Role.USER && (account?.stats.reservations_remaining || 0) > 0));

-  // load initial value, but update it in `handleBrowserNotificationsChanged`
-  // if we interact with the API and therefore possibly change it (from default -> denied)
-  const [notificationsExplicitlyDenied, setNotificationsExplicitlyDenied] = useState(notifier.denied());
-  // default to on if notifications are already granted
-  const [browserNotificationsEnabled, setBrowserNotificationsEnabled] = useState(notifier.granted());
-  const [backgroundNotificationsEnabled, setBackgroundNotificationsEnabled] = useState(
-    pushPossible && props.webPushDefaultEnabled === "enabled"
-  );
-
-  const handleBrowserNotificationsChanged = async (e) => {
-    if (e.target.checked && (await notifier.maybeRequestPermission())) {
-      setBrowserNotificationsEnabled(true);
-      if (pushPossible && props.webPushDefaultEnabled === "enabled") {
-        setBackgroundNotificationsEnabled(true);
-      }
-    } else {
-      setNotificationsExplicitlyDenied(notifier.denied());
-      setBrowserNotificationsEnabled(false);
-      setBackgroundNotificationsEnabled(false);
-    }
-  };
+  const [backgroundNotificationsEnabled, setBackgroundNotificationsEnabled] = useState(props.webPushDefaultEnabled === "enabled");

  const handleBackgroundNotificationsChanged = (e) => {
    setBackgroundNotificationsEnabled(e.target.checked);
@ -197,7 +158,7 @@ const SubscribePage = (props) => {
    }

    console.log(`[SubscribeDialog] Successful login to ${topicUrl(baseUrl, topic)} for user ${username}`);
-    props.onSuccess(getNotificationTypeFromToggles(browserNotificationsEnabled, backgroundNotificationsEnabled));
+    props.onSuccess(backgroundNotificationsEnabled);
  };

  const handleUseAnotherChanged = (e) => {
@ -311,41 +272,20 @@ const SubscribePage = (props) => {
            )}
          </FormGroup>
        )}
-        {browserNotificationsSupported && (
+        {notifier.pushPossible() && !anotherServerVisible && (
          <FormGroup>
            <FormControlLabel
              control={
                <Switch
-                  onChange={handleBrowserNotificationsChanged}
-                  checked={browserNotificationsEnabled}
-                  disabled={notificationsExplicitlyDenied}
+                  onChange={handleBackgroundNotificationsChanged}
+                  checked={backgroundNotificationsEnabled}
                  inputProps={{
-                    "aria-label": t("subscribe_dialog_subscribe_enable_browser_notifications_label"),
+                    "aria-label": t("subscribe_dialog_subscribe_enable_background_notifications_label"),
                  }}
                />
              }
-              label={
-                <Stack direction="row" gap={1} alignItems="center">
-                  {t("subscribe_dialog_subscribe_enable_browser_notifications_label")}
-                  {notificationsExplicitlyDenied && <Warning />}
-                </Stack>
-              }
+              label={t("subscribe_dialog_subscribe_enable_background_notifications_label")}
            />
-            {pushNotificationsSupported && !anotherServerVisible && browserNotificationsEnabled && (
-              <FormControlLabel
-                control={
-                  <Switch
-                    onChange={handleBackgroundNotificationsChanged}
-                    checked={backgroundNotificationsEnabled}
-                    disabled={iosInstallRequired}
-                    inputProps={{
-                      "aria-label": t("subscribe_dialog_subscribe_enable_background_notifications_label"),
-                    }}
-                  />
-                }
-                label={t("subscribe_dialog_subscribe_enable_background_notifications_label")}
-              />
-            )}
          </FormGroup>
        )}
      </DialogContent>
--- a/web/src/components/SubscriptionPopup.jsx
+++ b/web/src/components/SubscriptionPopup.jsx
@ -33,7 +33,7 @@ import {
  Send,
 } from "@mui/icons-material";
 import theme from "./theme";
-import subscriptionManager, { NotificationType } from "../app/SubscriptionManager";
+import subscriptionManager from "../app/SubscriptionManager";
 import DialogFooter from "./DialogFooter";
 import accountApi, { Role } from "../app/AccountApi";
 import session from "../app/Session";
@ -334,14 +334,6 @@ const DisplayNameDialog = (props) => {
  );
 };

-const getNotificationType = (subscription) => {
-  if (subscription.mutedUntil === 1) {
-    return "muted";
-  }
-
-  return subscription.notificationType ?? NotificationType.BROWSER;
-};
-
 const checkedItem = (
  <ListItemIcon>
    <Check />
@ -350,15 +342,10 @@ const checkedItem = (

 const NotificationToggle = ({ subscription }) => {
  const { t } = useTranslation();
-  const type = getNotificationType(subscription);

-  const handleChange = async (newType) => {
+  const handleToggleBackground = async () => {
    try {
-      if (newType !== NotificationType.SOUND && !(await notifier.maybeRequestPermission())) {
-        return;
-      }
-
-      await subscriptionManager.setNotificationType(subscription, newType);
+      await subscriptionManager.toggleBackgroundNotifications(subscription);
    } catch (e) {
      console.error("[NotificationToggle] Error setting notification type", e);
    }
@ -368,7 +355,7 @@ const NotificationToggle = ({ subscription }) => {
    await subscriptionManager.setMutedUntil(subscription.id, 0);
  };

-  if (type === "muted") {
+  if (subscription.mutedUntil === 1) {
    return (
      <MenuItem onClick={unmute}>
        <ListItemIcon>
@ -381,30 +368,14 @@ const NotificationToggle = ({ subscription }) => {

  return (
    <>
-      <MenuItem>
-        {type === NotificationType.SOUND && checkedItem}
-        <ListItemText inset={type !== NotificationType.SOUND} onClick={() => handleChange(NotificationType.SOUND)}>
-          {t("notification_toggle_sound")}
-        </ListItemText>
-      </MenuItem>
-      {!notifier.denied() && !notifier.iosSupportedButInstallRequired() && (
+      {notifier.pushPossible() && (
        <>
-          {notifier.supported() && (
-            <MenuItem>
-              {type === NotificationType.BROWSER && checkedItem}
-              <ListItemText inset={type !== NotificationType.BROWSER} onClick={() => handleChange(NotificationType.BROWSER)}>
-                {t("notification_toggle_browser")}
-              </ListItemText>
-            </MenuItem>
-          )}
-          {notifier.pushSupported() && (
-            <MenuItem>
-              {type === NotificationType.BACKGROUND && checkedItem}
-              <ListItemText inset={type !== NotificationType.BACKGROUND} onClick={() => handleChange(NotificationType.BACKGROUND)}>
-                {t("notification_toggle_background")}
-              </ListItemText>
-            </MenuItem>
-          )}
+          <MenuItem>
+            {subscription.webPushEnabled === 1 && checkedItem}
+            <ListItemText inset={subscription.webPushEnabled !== 1} onClick={handleToggleBackground}>
+              {t("notification_toggle_background")}
+            </ListItemText>
+          </MenuItem>
        </>
      )}
    </>
--- a/web/src/components/hooks.js
+++ b/web/src/components/hooks.js
@ -9,7 +9,8 @@ import pruner from "../app/Pruner";
 import session from "../app/Session";
 import accountApi from "../app/AccountApi";
 import { UnauthorizedError } from "../app/errors";
-import webPushWorker from "../app/WebPushWorker";
+import { webPushRefreshWorker, useWebPushUpdateWorker } from "../app/WebPushWorker";
+import notifier from "../app/Notifier";

 /**
 * Wire connectionManager and subscriptionManager so that subscriptions are updated when the connection
@ -134,24 +135,26 @@ const stopWorkers = () => {
  poller.stopWorker();
  pruner.stopWorker();
  accountApi.stopWorker();
+  webPushRefreshWorker.stopWorker();
 };

 const startWorkers = () => {
  poller.startWorker();
  pruner.startWorker();
  accountApi.startWorker();
+  webPushRefreshWorker.startWorker();
 };

 export const useBackgroundProcesses = () => {
+  useWebPushUpdateWorker();
+
  useEffect(() => {
    console.log("[useBackgroundProcesses] mounting");
    startWorkers();
-    webPushWorker.startWorker();

    return () => {
      console.log("[useBackgroundProcesses] unloading");
      stopWorkers();
-      webPushWorker.stopWorker();
    };
  }, []);
 };