From e4e98286f445b1f799c783f703b437aca01d04fe Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sun, 17 Apr 2022 10:24:40 -0700 Subject: [PATCH] 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 | 21 +-- .../Sidebar/components/Collections.tsx | 13 +- app/components/Sidebar/components/Header.tsx | 64 ++++++-- .../Sidebar/components/SidebarLink.tsx | 16 +- app/components/Sidebar/components/Starred.tsx | 140 +++++------------- app/hooks/usePersistedState.ts | 51 +++++++ app/models/Document.ts | 30 ++-- app/stores/AuthStore.ts | 34 ++--- app/stores/UiStore.ts | 26 +--- app/utils/Storage.ts | 56 +++++++ shared/editor/nodes/Embed.tsx | 8 - 11 files changed, 267 insertions(+), 192 deletions(-) create mode 100644 app/hooks/usePersistedState.ts create mode 100644 app/utils/Storage.ts diff --git a/app/components/Sidebar/Settings.tsx b/app/components/Sidebar/Settings.tsx index c190963f4..83f0ad942 100644 --- a/app/components/Sidebar/Settings.tsx +++ b/app/components/Sidebar/Settings.tsx @@ -39,20 +39,21 @@ function SettingsSidebar() { {Object.keys(groupedConfig).map((header) => (
-
{header}
- {groupedConfig[header].map((item) => ( - } - label={item.name} - /> - ))} +
+ {groupedConfig[header].map((item) => ( + } + label={item.name} + /> + ))} +
))} {!isHosted && (
-
{t("Installation")}
+
)} diff --git a/app/components/Sidebar/components/Collections.tsx b/app/components/Sidebar/components/Collections.tsx index b2311d2d9..c559993f0 100644 --- a/app/components/Sidebar/components/Collections.tsx +++ b/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 ( -
{t("Collections")}
- +
+ +
); } return ( -
setExpanded((prev) => !prev)} expanded={expanded}> - {t("Collections")} -
- {expanded && ( +
{isPreloaded ? content : {content}} - )} +
); } diff --git a/app/components/Sidebar/components/Header.tsx b/app/components/Sidebar/components/Header.tsx index e779b3b46..c5f84a499 100644 --- a/app/components/Sidebar/components/Header.tsx +++ b/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; - expanded?: boolean; + /** Unique header id – if passed the header will become toggleable */ + id?: string; + title: React.ReactNode; }; -export const Header: React.FC = ({ onClick, expanded, children }) => { +/** + * Toggleable sidebar header + */ +export const Header: React.FC = ({ 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 ( -

- -

+ <> +

+ +

+ {expanded && (firstRender ? children : {children})} + ); }; +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; diff --git a/app/components/Sidebar/components/SidebarLink.tsx b/app/components/Sidebar/components/SidebarLink.tsx index 0f0c04e30..4c132f72f 100644 --- a/app/components/Sidebar/components/SidebarLink.tsx +++ b/app/components/Sidebar/components/SidebarLink.tsx @@ -24,6 +24,7 @@ type Props = Omit & { 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 @@ -82,6 +84,7 @@ function SidebarLink( ` } `; -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` diff --git a/app/components/Sidebar/components/Starred.tsx b/app/components/Sidebar/components/Starred.tsx index e9d84a80b..1b4b6cead 100644 --- a/app/components/Sidebar/components/Starred.tsx +++ b/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 [show, setShow] = React.useState("Nothing"); - const [offset, setOffset] = React.useState(0); - const [upperBound, setUpperBound] = React.useState(STARRED_PAGINATION_LIMIT); + const [displayedStarsCount, setDisplayedStarsCount] = React.useState( + STARRED_PAGINATION_LIMIT + ); const { showToast } = useToasts(); const { stars } = useStores(); const { t } = useTranslation(); - const fetchResults = React.useCallback(async () => { - try { - 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(); - + const fetchResults = React.useCallback( + async (offset = 0) => { try { - localStorage.setItem(STARRED, !expanded ? "true" : "false"); - } catch (_) { - // no-op Safari private mode + await stars.fetchPage({ + limit: STARRED_PAGINATION_LIMIT + 1, + 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 ( -
- {t("Starred")} -
- {expanded && ( +
- - {stars.orderedData.slice(0, upperBound).map((star) => ( + {isDraggingAnyStar && ( + + )} + {stars.orderedData.slice(0, displayedStarsCount).map((star) => ( ))} - {show === "More" && !isFetching && ( + {stars.orderedData.length > displayedStarsCount && ( )} - {show === "Less" && !isFetching && ( - - )} - {(isFetching || fetchError) && !stars.orderedData.length && ( + {(stars.isFetching || fetchError) && !stars.orderedData.length && ( )} - )} +
); diff --git a/app/hooks/usePersistedState.ts b/app/hooks/usePersistedState.ts new file mode 100644 index 000000000..42e0a51e4 --- /dev/null +++ b/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]; +} diff --git a/app/models/Document.ts b/app/models/Document.ts index 75df7c992..dd4bc375a 100644 --- a/app/models/Document.ts +++ b/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, 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, store: DocumentsStore) { - super(fields, store); - - if (this.isPersistedOnce && this.isFromTemplate) { - this.title = ""; - } - } - @computed get emoji() { const { emoji } = parseTitle(this.title); diff --git a/app/stores/AuthStore.ts b/app/stores/AuthStore.ts index 89e65f64b..aedfb4536 100644 --- a/app/stores/AuthStore.ts +++ b/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 = {}; - - try { - data = JSON.parse(localStorage.getItem(AUTH_STORE) || "{}"); - } catch (err) { - Sentry.captureException(err); - } + const data: PersistedData = Storage.get(AUTH_STORE) || {}; this.rehydrate(data); // persists this entire store to localstorage whenever any keys are changed autorun(() => { - try { - localStorage.setItem(AUTH_STORE, this.asJson); - } catch (err) { - Sentry.captureException(err); - } + Storage.set(AUTH_STORE, this.asJson); }); // listen to the localstorage value changing in other tabs to react to @@ -135,12 +126,12 @@ export default class AuthStore { } @computed - get asJson(): string { - return JSON.stringify({ + get asJson() { + 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( - AUTH_STORE, - JSON.stringify({ - user: null, - team: null, - policies: [], - }) - ); + Storage.set(AUTH_STORE, { + user: null, + team: null, + policies: [], + }); this.token = null; // if this logout was forced from an authenticated route then diff --git a/app/stores/UiStore.ts b/app/stores/UiStore.ts index bef203fa3..8186dd803 100644 --- a/app/stores/UiStore.ts +++ b/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 = {}; - - try { - data = JSON.parse(localStorage.getItem(UI_STORE) || "{}"); - } catch (_) { - // no-op Safari private mode - } + const data: Partial = Storage.get(UI_STORE) || {}; // system theme listeners if (window.matchMedia) { @@ -100,21 +95,14 @@ class UiStore { this.theme = data.theme || Theme.System; autorun(() => { - try { - localStorage.setItem(UI_STORE, this.asJson); - } catch (_) { - // no-op Safari private mode - } + Storage.set(UI_STORE, this.asJson); }); } @action setTheme = (theme: Theme) => { this.theme = theme; - - if (window.localStorage) { - window.localStorage.setItem("theme", this.theme); - } + Storage.set("theme", this.theme); }; @action @@ -244,14 +232,14 @@ class UiStore { } @computed - get asJson(): string { - return JSON.stringify({ + get asJson() { + return { tocVisible: this.tocVisible, sidebarCollapsed: this.sidebarCollapsed, sidebarWidth: this.sidebarWidth, languagePromptDismissed: this.languagePromptDismissed, theme: this.theme, - }); + }; } } diff --git a/app/utils/Storage.ts b/app/utils/Storage.ts new file mode 100644 index 000000000..a6e86719f --- /dev/null +++ b/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(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 + } + } +} diff --git a/shared/editor/nodes/Embed.tsx b/shared/editor/nodes/Embed.tsx index 8109786f6..87b74a173 100644 --- a/shared/editor/nodes/Embed.tsx +++ b/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",