fix: Embed disabled state should persist (#3407)

* Normalize code around localStorage
Persist disabled embed state

* fix: Cannot view more than 10 starred items on load

* More tidying of sidebar state

app/components/Sidebar/Settings.tsx

@@ -39,20 +39,21 @@ function SettingsSidebar() {
         <Scrollable shadow>
           {Object.keys(groupedConfig).map((header) => (
             <Section key={header}>
-              <Header>{header}</Header>
+              <Header title={header}>
-              {groupedConfig[header].map((item) => (
+                {groupedConfig[header].map((item) => (
-                <SidebarLink
+                  <SidebarLink
-                  key={item.path}
+                    key={item.path}
-                  to={item.path}
+                    to={item.path}
-                  icon={<item.icon color="currentColor" />}
+                    icon={<item.icon color="currentColor" />}
-                  label={item.name}
+                    label={item.name}
-                />
+                  />
-              ))}
+                ))}
+              </Header>
             </Section>
           ))}
           {!isHosted && (
             <Section>
-              <Header>{t("Installation")}</Header>
+              <Header title={t("Installation")} />
               <Version />
             </Section>
           )}

app/components/Sidebar/components/Collections.tsx

@@ -22,7 +22,6 @@ function Collections() {
   const [fetchError, setFetchError] = React.useState();
   const { documents, collections } = useStores();
   const { showToast } = useToasts();
-  const [expanded, setExpanded] = React.useState(true);
   const isPreloaded = !!collections.orderedData.length;
   const { t } = useTranslation();
   const orderedCollections = collections.orderedData;
@@ -97,20 +96,18 @@ function Collections() {
   if (!collections.isLoaded || fetchError) {
     return (
       <Flex column>
-        <Header>{t("Collections")}</Header>
+        <Header id="collections" title={t("Collections")}>
-        <PlaceholderCollections />
+          <PlaceholderCollections />
+        </Header>
       </Flex>
     );
   }
 
   return (
     <Flex column>
-      <Header onClick={() => setExpanded((prev) => !prev)} expanded={expanded}>
+      <Header id="collections" title={t("Collections")}>
-        {t("Collections")}
-      </Header>
-      {expanded && (
         <Relative>{isPreloaded ? content : <Fade>{content}</Fade>}</Relative>
-      )}
+      </Header>
     </Flex>
   );
 }

app/components/Sidebar/components/Header.tsx

@@ -1,25 +1,65 @@
 import { CollapsedIcon } from "outline-icons";
 import * as React from "react";
-import styled from "styled-components";
+import styled, { keyframes } from "styled-components";
+import usePersistedState from "~/hooks/usePersistedState";
 
 type Props = {
-  onClick?: React.MouseEventHandler;
+  /** Unique header id – if passed the header will become toggleable */
-  expanded?: boolean;
+  id?: string;
+  title: React.ReactNode;
 };
 
-export const Header: React.FC<Props> = ({ onClick, expanded, children }) => {
+/**
+ * Toggleable sidebar header
+ */
+export const Header: React.FC<Props> = ({ id, title, children }) => {
+  const [firstRender, setFirstRender] = React.useState(true);
+  const [expanded, setExpanded] = usePersistedState(
+    `sidebar-header-${id}`,
+    true
+  );
+
+  React.useEffect(() => {
+    if (!expanded) {
+      setFirstRender(false);
+    }
+  }, [expanded]);
+
+  const handleClick = React.useCallback(() => {
+    setExpanded(!expanded);
+  }, [expanded, setExpanded]);
+
   return (
-    <H3>
+    <>
-      <Button onClick={onClick} disabled={!onClick}>
+      <H3>
-        {children}
+        <Button onClick={handleClick} disabled={!id}>
-        {onClick && (
+          {title}
-          <Disclosure expanded={expanded} color="currentColor" size={20} />
+          {id && (
-        )}
+            <Disclosure expanded={expanded} color="currentColor" size={20} />
-      </Button>
+          )}
-    </H3>
+        </Button>
+      </H3>
+      {expanded && (firstRender ? children : <Fade>{children}</Fade>)}
+    </>
   );
 };
 
+export const fadeAndSlideDown = keyframes`
+  from {
+    opacity: 0;
+    transform: translateY(-8px);
+  }
+
+  to {
+    opacity: 1;
+    transform: translateY(0px);
+  }
+`;
+
+const Fade = styled.span`
+  animation: ${fadeAndSlideDown} 100ms ease-in-out;
+`;
+
 const Button = styled.button`
   display: inline-flex;
   align-items: center;

app/components/Sidebar/components/SidebarLink.tsx

@@ -24,6 +24,7 @@ type Props = Omit<NavLinkProps, "to"> & {
   label?: React.ReactNode;
   menu?: React.ReactNode;
   showActions?: boolean;
+  disabled?: boolean;
   active?: boolean;
   /* If set, a disclosure will be rendered to the left of any icon */
   expanded?: boolean;
@@ -55,6 +56,7 @@ function SidebarLink(
     className,
     expanded,
     onDisclosureClick,
+    disabled,
     ...rest
   }: Props,
   ref: React.RefObject<HTMLAnchorElement>
@@ -82,6 +84,7 @@ function SidebarLink(
       <Link
         $isActiveDrop={isActiveDrop}
         $isDraft={isDraft}
+        $disabled={disabled}
         activeStyle={isActiveDrop ? activeDropStyle : activeStyle}
         style={active ? activeStyle : style}
         onClick={onClick}
@@ -158,7 +161,11 @@ const Actions = styled(EventBoundary)<{ showActions?: boolean }>`
   }
 `;
 
-const Link = styled(NavLink)<{ $isActiveDrop?: boolean; $isDraft?: boolean }>`
+const Link = styled(NavLink)<{
+  $isActiveDrop?: boolean;
+  $isDraft?: boolean;
+  $disabled?: boolean;
+}>`
   display: flex;
   position: relative;
   text-overflow: ellipsis;
@@ -174,6 +181,13 @@ const Link = styled(NavLink)<{ $isActiveDrop?: boolean; $isDraft?: boolean }>`
   cursor: pointer;
   overflow: hidden;
 
+  ${(props) =>
+    props.$disabled &&
+    css`
+      pointer-events: none;
+      opacity: 0.75;
+    `}
+
   ${(props) =>
     props.$isDraft &&
     css`

app/components/Sidebar/components/Starred.tsx

@@ -16,106 +16,51 @@ import StarredContext from "./StarredContext";
 import StarredLink from "./StarredLink";
 
 const STARRED_PAGINATION_LIMIT = 10;
-const STARRED = "STARRED";
 
 function Starred() {
-  const [isFetching, setIsFetching] = React.useState(false);
   const [fetchError, setFetchError] = React.useState();
-  const [expanded, setExpanded] = React.useState(true);
+  const [displayedStarsCount, setDisplayedStarsCount] = React.useState(
-  const [show, setShow] = React.useState("Nothing");
+    STARRED_PAGINATION_LIMIT
-  const [offset, setOffset] = React.useState(0);
+  );
-  const [upperBound, setUpperBound] = React.useState(STARRED_PAGINATION_LIMIT);
   const { showToast } = useToasts();
   const { stars } = useStores();
   const { t } = useTranslation();
 
-  const fetchResults = React.useCallback(async () => {
+  const fetchResults = React.useCallback(
-    try {
+    async (offset = 0) => {
-      setIsFetching(true);
-      await stars.fetchPage({
-        limit: STARRED_PAGINATION_LIMIT,
-        offset,
-      });
-    } catch (error) {
-      showToast(t("Starred documents could not be loaded"), {
-        type: "error",
-      });
-      setFetchError(error);
-    } finally {
-      setIsFetching(false);
-    }
-  }, [stars, offset, showToast, t]);
-
-  React.useEffect(() => {
-    let stateInLocal;
-
-    try {
-      stateInLocal = localStorage.getItem(STARRED);
-    } catch (_) {
-      // no-op Safari private mode
-    }
-
-    if (!stateInLocal) {
-      localStorage.setItem(STARRED, expanded ? "true" : "false");
-    } else {
-      setExpanded(stateInLocal === "true");
-    }
-  }, [expanded]);
-
-  React.useEffect(() => {
-    setOffset(stars.orderedData.length);
-
-    if (stars.orderedData.length <= STARRED_PAGINATION_LIMIT) {
-      setShow("Nothing");
-    } else if (stars.orderedData.length >= upperBound) {
-      setShow("More");
-    } else if (stars.orderedData.length < upperBound) {
-      setShow("Less");
-    }
-  }, [stars.orderedData, upperBound]);
-
-  React.useEffect(() => {
-    if (offset === 0) {
-      fetchResults();
-    }
-  }, [fetchResults, offset]);
-
-  const handleShowMore = React.useCallback(async () => {
-    setUpperBound(
-      (previousUpperBound) => previousUpperBound + STARRED_PAGINATION_LIMIT
-    );
-    await fetchResults();
-  }, [fetchResults]);
-
-  const handleShowLess = React.useCallback(() => {
-    setUpperBound(STARRED_PAGINATION_LIMIT);
-    setShow("More");
-  }, []);
-
-  const handleExpandClick = React.useCallback(
-    (ev) => {
-      ev.preventDefault();
-      ev.stopPropagation();
-
       try {
-        localStorage.setItem(STARRED, !expanded ? "true" : "false");
+        await stars.fetchPage({
-      } catch (_) {
+          limit: STARRED_PAGINATION_LIMIT + 1,
-        // no-op Safari private mode
+          offset,
+        });
+      } catch (error) {
+        showToast(t("Starred documents could not be loaded"), {
+          type: "error",
+        });
+        setFetchError(error);
       }
-
-      setExpanded((prev) => !prev);
     },
-    [expanded]
+    [stars, showToast, t]
   );
 
+  React.useEffect(() => {
+    fetchResults();
+  }, [fetchResults]);
+
+  const handleShowMore = async () => {
+    await fetchResults(displayedStarsCount);
+    setDisplayedStarsCount((prev) => prev + STARRED_PAGINATION_LIMIT);
+  };
+
   // Drop to reorder document
-  const [{ isOverReorder }, dropToReorder] = useDrop({
+  const [{ isOverReorder, isDraggingAnyStar }, dropToReorder] = useDrop({
     accept: "star",
     drop: async (item: Star) => {
       item?.save({ index: fractionalIndex(null, stars.orderedData[0].index) });
     },
     collect: (monitor) => ({
       isOverReorder: !!monitor.isOver(),
+      isDraggingAnyStar: monitor.getItemType() === "star",
     }),
   });
 
@@ -126,40 +71,33 @@ function Starred() {
   return (
     <StarredContext.Provider value={true}>
       <Flex column>
-        <Header onClick={handleExpandClick} expanded={expanded}>
+        <Header id="starred" title={t("Starred")}>
-          {t("Starred")}
-        </Header>
-        {expanded && (
           <Relative>
-            <DropCursor
+            {isDraggingAnyStar && (
-              isActiveDrop={isOverReorder}
+              <DropCursor
-              innerRef={dropToReorder}
+                isActiveDrop={isOverReorder}
-              position="top"
+                innerRef={dropToReorder}
-            />
+                position="top"
-            {stars.orderedData.slice(0, upperBound).map((star) => (
+              />
+            )}
+            {stars.orderedData.slice(0, displayedStarsCount).map((star) => (
               <StarredLink key={star.id} star={star} />
             ))}
-            {show === "More" && !isFetching && (
+            {stars.orderedData.length > displayedStarsCount && (
               <SidebarLink
                 onClick={handleShowMore}
                 label={`${t("Show more")}…`}
+                disabled={stars.isFetching}
                 depth={0}
               />
             )}
-            {show === "Less" && !isFetching && (
+            {(stars.isFetching || fetchError) && !stars.orderedData.length && (
-              <SidebarLink
-                onClick={handleShowLess}
-                label={`${t("Show less")}…`}
-                depth={0}
-              />
-            )}
-            {(isFetching || fetchError) && !stars.orderedData.length && (
               <Flex column>
                 <PlaceholderCollections />
               </Flex>
             )}
           </Relative>
-        )}
+        </Header>
       </Flex>
     </StarredContext.Provider>
   );

app/hooks/usePersistedState.ts

@@ -0,0 +1,51 @@
+import * as React from "react";
+import { Primitive } from "utility-types";
+import Storage from "~/utils/Storage";
+
+/**
+ * A hook with the same API as `useState` that persists its value locally and
+ * syncs the value between browser tabs.
+ *
+ * @param key Key to store value under
+ * @param defaultValue An optional default value if no key exists
+ * @returns Tuple of the current value and a function to update it
+ */
+export default function usePersistedState(
+  key: string,
+  defaultValue: Primitive
+) {
+  const [storedValue, setStoredValue] = React.useState(() => {
+    if (typeof window === "undefined") {
+      return defaultValue;
+    }
+    return Storage.get(key) ?? defaultValue;
+  });
+
+  const setValue = (value: Primitive | ((value: Primitive) => void)) => {
+    try {
+      // Allow value to be a function so we have same API as useState
+      const valueToStore =
+        value instanceof Function ? value(storedValue) : value;
+
+      setStoredValue(valueToStore);
+      Storage.set(key, valueToStore);
+    } catch (error) {
+      // A more advanced implementation would handle the error case
+      console.log(error);
+    }
+  };
+
+  // Listen to the key changing in other tabs so we can keep UI in sync
+  React.useEffect(() => {
+    const updateValue = (event: any) => {
+      if (event.key === key && event.newValue) {
+        setStoredValue(JSON.parse(event.newValue));
+      }
+    };
+
+    window.addEventListener("storage", updateValue);
+    return () => window.removeEventListener("storage", updateValue);
+  }, [key]);
+
+  return [storedValue, setValue];
+}

app/models/Document.ts

@@ -1,11 +1,12 @@
 import { addDays, differenceInDays } from "date-fns";
 import { floor } from "lodash";
-import { action, computed, observable } from "mobx";
+import { action, autorun, computed, observable } from "mobx";
 import parseTitle from "@shared/utils/parseTitle";
 import unescape from "@shared/utils/unescape";
 import DocumentsStore from "~/stores/DocumentsStore";
 import User from "~/models/User";
 import { NavigationNode } from "~/types";
+import Storage from "~/utils/Storage";
 import ParanoidModel from "./ParanoidModel";
 import View from "./View";
 import Field from "./decorators/Field";
@@ -18,11 +19,28 @@ type SaveOptions = {
 };
 
 export default class Document extends ParanoidModel {
+  constructor(fields: Record<string, any>, store: DocumentsStore) {
+    super(fields, store);
+
+    if (this.isPersistedOnce && this.isFromTemplate) {
+      this.title = "";
+    }
+
+    this.embedsDisabled = Storage.get(`embedsDisabled-${this.id}`) ?? false;
+
+    autorun(() => {
+      Storage.set(
+        `embedsDisabled-${this.id}`,
+        this.embedsDisabled ? true : undefined
+      );
+    });
+  }
+
   @observable
   isSaving = false;
 
   @observable
-  embedsDisabled = false;
+  embedsDisabled: boolean;
 
   @observable
   lastViewedAt: string | undefined;
@@ -82,14 +100,6 @@ export default class Document extends ParanoidModel {
 
   revision: number;
 
-  constructor(fields: Record<string, any>, store: DocumentsStore) {
-    super(fields, store);
-
-    if (this.isPersistedOnce && this.isFromTemplate) {
-      this.title = "";
-    }
-  }
-
   @computed
   get emoji() {
     const { emoji } = parseTitle(this.title);

app/stores/AuthStore.ts

@@ -8,6 +8,7 @@ import Team from "~/models/Team";
 import User from "~/models/User";
 import env from "~/env";
 import { client } from "~/utils/ApiClient";
+import Storage from "~/utils/Storage";
 import { getCookieDomain } from "~/utils/domains";
 
 const AUTH_STORE = "AUTH_STORE";
@@ -64,23 +65,13 @@ export default class AuthStore {
   constructor(rootStore: RootStore) {
     this.rootStore = rootStore;
     // attempt to load the previous state of this store from localstorage
-    let data: PersistedData = {};
+    const data: PersistedData = Storage.get(AUTH_STORE) || {};
-
-    try {
-      data = JSON.parse(localStorage.getItem(AUTH_STORE) || "{}");
-    } catch (err) {
-      Sentry.captureException(err);
-    }
 
     this.rehydrate(data);
 
     // persists this entire store to localstorage whenever any keys are changed
     autorun(() => {
-      try {
+      Storage.set(AUTH_STORE, this.asJson);
-        localStorage.setItem(AUTH_STORE, this.asJson);
-      } catch (err) {
-        Sentry.captureException(err);
-      }
     });
 
     // listen to the localstorage value changing in other tabs to react to
@@ -135,12 +126,12 @@ export default class AuthStore {
   }
 
   @computed
-  get asJson(): string {
+  get asJson() {
-    return JSON.stringify({
+    return {
       user: this.user,
       team: this.team,
       policies: this.policies,
-    });
+    };
   }
 
   @action
@@ -248,14 +239,11 @@ export default class AuthStore {
   @action
   logout = async (savePath = false) => {
     // remove user and team from localStorage
-    localStorage.setItem(
+    Storage.set(AUTH_STORE, {
-      AUTH_STORE,
+      user: null,
-      JSON.stringify({
+      team: null,
-        user: null,
+      policies: [],
-        team: null,
+    });
-        policies: [],
-      })
-    );
     this.token = null;
 
     // if this logout was forced from an authenticated route then

app/stores/UiStore.ts

@@ -2,6 +2,7 @@ import { action, autorun, computed, observable } from "mobx";
 import { light as defaultTheme } from "@shared/styles/theme";
 import Document from "~/models/Document";
 import { ConnectionStatus } from "~/scenes/Document/components/MultiplayerEditor";
+import Storage from "~/utils/Storage";
 
 const UI_STORE = "UI_STORE";
 
@@ -67,13 +68,7 @@ class UiStore {
 
   constructor() {
     // Rehydrate
-    let data: Partial<UiStore> = {};
+    const data: Partial<UiStore> = Storage.get(UI_STORE) || {};
-
-    try {
-      data = JSON.parse(localStorage.getItem(UI_STORE) || "{}");
-    } catch (_) {
-      // no-op Safari private mode
-    }
 
     // system theme listeners
     if (window.matchMedia) {
@@ -100,21 +95,14 @@ class UiStore {
     this.theme = data.theme || Theme.System;
 
     autorun(() => {
-      try {
+      Storage.set(UI_STORE, this.asJson);
-        localStorage.setItem(UI_STORE, this.asJson);
-      } catch (_) {
-        // no-op Safari private mode
-      }
     });
   }
 
   @action
   setTheme = (theme: Theme) => {
     this.theme = theme;
-
+    Storage.set("theme", this.theme);
-    if (window.localStorage) {
-      window.localStorage.setItem("theme", this.theme);
-    }
   };
 
   @action
@@ -244,14 +232,14 @@ class UiStore {
   }
 
   @computed
-  get asJson(): string {
+  get asJson() {
-    return JSON.stringify({
+    return {
       tocVisible: this.tocVisible,
       sidebarCollapsed: this.sidebarCollapsed,
       sidebarWidth: this.sidebarWidth,
       languagePromptDismissed: this.languagePromptDismissed,
       theme: this.theme,
-    });
+    };
   }
 }
 

app/utils/Storage.ts

@@ -0,0 +1,56 @@
+/**
+ * Storage is a wrapper class for localStorage that allow safe usage when
+ * localStorage is not available.
+ */
+export default class Storage {
+  /**
+   * Set a value in localStorage. For efficiency, this method will remove the
+   * value if it is undefined.
+   *
+   * @param key The key to set under.
+   * @param value The value to set
+   */
+  static set<T>(key: string, value: T) {
+    try {
+      if (value === undefined) {
+        this.remove(key);
+      } else {
+        localStorage.setItem(key, JSON.stringify(value));
+      }
+    } catch (error) {
+      // no-op Safari private mode
+    }
+  }
+
+  /**
+   * Get a value from localStorage.
+   *
+   * @param key The key to get.
+   * @returns The value or undefined if it doesn't exist.
+   */
+  static get(key: string) {
+    try {
+      const value = localStorage.getItem(key);
+      if (typeof value === "string") {
+        return JSON.parse(value);
+      }
+    } catch (error) {
+      // no-op Safari private mode
+    }
+
+    return undefined;
+  }
+
+  /**
+   * Remove a value from localStorage.
+   *
+   * @param key The key to remove.
+   */
+  static remove(key: string) {
+    try {
+      localStorage.removeItem(key);
+    } catch (error) {
+      // no-op Safari private mode
+    }
+  }
+}

shared/editor/nodes/Embed.tsx

@@ -44,14 +44,6 @@ export default class Embed extends Node {
             return {};
           },
         },
-        {
-          tag: "a.disabled-embed",
-          getAttrs: (dom: HTMLAnchorElement) => {
-            return {
-              href: dom.getAttribute("href") || "",
-            };
-          },
-        },
       ],
       toDOM: (node) => [
         "iframe",