Revert "fix: harden automatic mobile boards"
This reverts commit d0b9348b86.
This commit is contained in:
1 parent
92a2be2b65
commit
dba9a250ff
37 files changed
+137
-637
No files matched your search
@@ -1,51 +0,0 @@
|
||||
import { beforeEach, describe, expect, test, vi } from "vitest";
|
||||
|
||||
import { classifyGuardedNavigation, removeGuardHistoryEntry, replaceGuardedSameDocumentUrl } from "./_guarded-history";
|
||||
|
||||
describe("guarded board edit history", () => {
|
||||
beforeEach(() => {
|
||||
window.history.replaceState({ page: "board" }, "", "/boards/demo");
|
||||
});
|
||||
|
||||
test("replaces the guard entry for same-document fragment navigation", () => {
|
||||
const section = document.createElement("section");
|
||||
section.id = "Media requests";
|
||||
document.body.appendChild(section);
|
||||
const scrollIntoView = vi.fn();
|
||||
Object.defineProperty(section, "scrollIntoView", { value: scrollIntoView });
|
||||
|
||||
replaceGuardedSameDocumentUrl(new URL("/boards/demo#Media%20requests", window.location.href), {
|
||||
key: "__guard",
|
||||
id: "guard-id",
|
||||
});
|
||||
|
||||
expect(window.location.hash).toBe("#Media%20requests");
|
||||
expect(window.history.state).toStrictEqual({
|
||||
page: "board",
|
||||
__guard: "guard-id",
|
||||
});
|
||||
expect(scrollIntoView).toHaveBeenCalledOnce();
|
||||
section.remove();
|
||||
});
|
||||
|
||||
test("treats an explicit empty fragment as same-document navigation", () => {
|
||||
const current = new URL(window.location.href);
|
||||
const destination = new URL("#", current);
|
||||
|
||||
expect(classifyGuardedNavigation(destination, current)).toBe("same-document");
|
||||
});
|
||||
|
||||
test("removes the guard while preserving the fragment when editing ends", () => {
|
||||
window.history.replaceState({ page: "board", __guard: "guard-id" }, "", "/boards/demo#section");
|
||||
const back = vi.spyOn(window.history, "back").mockImplementation(() => {
|
||||
window.history.replaceState({ page: "board" }, "", "/boards/demo");
|
||||
window.dispatchEvent(new PopStateEvent("popstate"));
|
||||
});
|
||||
|
||||
removeGuardHistoryEntry("http://localhost:3000/boards/demo#section", "__guard");
|
||||
|
||||
expect(back).toHaveBeenCalledOnce();
|
||||
expect(window.location.href).toBe("http://localhost:3000/boards/demo#section");
|
||||
expect(window.history.state).toStrictEqual({ page: "board" });
|
||||
});
|
||||
});
|
||||
@@ -1,58 +0,0 @@
|
||||
const getHistoryState = () =>
|
||||
window.history.state && typeof window.history.state === "object"
|
||||
? (window.history.state as Record<string, unknown>)
|
||||
: {};
|
||||
|
||||
interface GuardState {
|
||||
key: string;
|
||||
id: string;
|
||||
}
|
||||
|
||||
export const classifyGuardedNavigation = (destination: URL, current: URL) => {
|
||||
if (destination.href === current.href) return "same-url";
|
||||
if (
|
||||
destination.origin === current.origin &&
|
||||
destination.pathname === current.pathname &&
|
||||
destination.search === current.search
|
||||
) {
|
||||
return "same-document";
|
||||
}
|
||||
return "other-document";
|
||||
};
|
||||
|
||||
export const replaceGuardedSameDocumentUrl = (destination: URL, guard: GuardState) => {
|
||||
window.history.replaceState(
|
||||
{
|
||||
...getHistoryState(),
|
||||
[guard.key]: guard.id,
|
||||
},
|
||||
document.title,
|
||||
destination.href,
|
||||
);
|
||||
|
||||
if (!destination.hash) {
|
||||
window.scrollTo({ top: 0, left: 0 });
|
||||
return;
|
||||
}
|
||||
|
||||
let targetId = destination.hash.slice(1);
|
||||
try {
|
||||
targetId = decodeURIComponent(targetId);
|
||||
} catch {
|
||||
// Keep the encoded value when the fragment contains malformed escape sequences.
|
||||
}
|
||||
document.getElementById(targetId)?.scrollIntoView();
|
||||
};
|
||||
|
||||
export const removeGuardHistoryEntry = (guardedUrl: string, guardStateKey: string) => {
|
||||
window.addEventListener(
|
||||
"popstate",
|
||||
() => {
|
||||
const state = { ...getHistoryState() };
|
||||
delete state[guardStateKey];
|
||||
window.history.replaceState(state, document.title, guardedUrl);
|
||||
},
|
||||
{ once: true },
|
||||
);
|
||||
window.history.back();
|
||||
};
|
||||
@@ -55,8 +55,6 @@ import { CurrentColorSchemeCombobox } from "~/components/color-scheme/current-co
|
||||
import { CurrentLanguageCombobox } from "~/components/language/current-language-combobox";
|
||||
import { HeaderButton } from "~/components/layout/header/button";
|
||||
|
||||
import { classifyGuardedNavigation, removeGuardHistoryEntry, replaceGuardedSameDocumentUrl } from "./_guarded-history";
|
||||
|
||||
export const BoardContentHeaderActions = ({ demoReadOnly }: { demoReadOnly: boolean }) => {
|
||||
const [isEditMode] = useEditMode();
|
||||
const board = useRequiredBoard();
|
||||
@@ -76,9 +74,9 @@ export const BoardContentHeaderActions = ({ demoReadOnly }: { demoReadOnly: bool
|
||||
|
||||
return (
|
||||
<>
|
||||
{isEditMode && <AddMenu />}
|
||||
{isEditMode && !isMobile && <AddMenu />}
|
||||
|
||||
<EditModeMenu demoReadOnly={demoReadOnly} />
|
||||
<EditModeMenu demoReadOnly={demoReadOnly} hidden={isMobile} />
|
||||
|
||||
{!demoReadOnly && (
|
||||
<OnboardingTour.Target id="board-settings">
|
||||
@@ -103,7 +101,6 @@ const AddMenu = () => {
|
||||
const { addDynamicSection } = useDynamicSectionActions();
|
||||
const { createItem } = useItemActions();
|
||||
const t = useI18n();
|
||||
const canCreateApp = session?.user.permissions.includes("app-create") ?? false;
|
||||
|
||||
const handleAddCategory = useCallback(
|
||||
() =>
|
||||
@@ -119,7 +116,7 @@ const AddMenu = () => {
|
||||
submitLabel: t("section.category.create.submit"),
|
||||
},
|
||||
{
|
||||
title: (translate) => translate("section.category.create.title"),
|
||||
title: (t) => t("section.category.create.title"),
|
||||
},
|
||||
),
|
||||
[addCategoryToEnd, openCategoryEditModal, t],
|
||||
@@ -137,9 +134,9 @@ const AddMenu = () => {
|
||||
options: { appId: app.id },
|
||||
});
|
||||
},
|
||||
withCreate: canCreateApp,
|
||||
withCreate: session?.user.permissions.includes("app-create") ?? false,
|
||||
});
|
||||
}, [openAppSelectModal, createItem, canCreateApp]);
|
||||
}, [openAppSelectModal, createItem]);
|
||||
|
||||
const handleAddIntegration = useCallback(() => {
|
||||
openIntegrationSelectModal({});
|
||||
@@ -182,7 +179,7 @@ const AddMenu = () => {
|
||||
);
|
||||
};
|
||||
|
||||
const EditModeMenu = ({ demoReadOnly }: { demoReadOnly: boolean }) => {
|
||||
const EditModeMenu = ({ demoReadOnly, hidden }: { demoReadOnly: boolean; hidden: boolean }) => {
|
||||
const [isEditMode, { open, close }] = useEditMode();
|
||||
const board = useRequiredBoard();
|
||||
const utils = clientApi.useUtils();
|
||||
@@ -212,14 +209,16 @@ const EditModeMenu = ({ demoReadOnly }: { demoReadOnly: boolean }) => {
|
||||
}, [utils, board.name, close]);
|
||||
|
||||
const toggle = useCallback(() => {
|
||||
if (hidden) return;
|
||||
if (isEditMode) {
|
||||
if (demoReadOnly) return discardDemoChanges();
|
||||
return saveBoard(board);
|
||||
}
|
||||
open();
|
||||
}, [board, isEditMode, demoReadOnly, saveBoard, open, discardDemoChanges]);
|
||||
}, [board, isEditMode, demoReadOnly, saveBoard, open, discardDemoChanges, hidden]);
|
||||
|
||||
useHotkeys([[hotkeys.toggleBoardEdit, toggle]]);
|
||||
if (hidden) return null;
|
||||
|
||||
return (
|
||||
<OnboardingTour.Target id="board-edit-mode">
|
||||
@@ -567,18 +566,8 @@ const usePreventLeaveWithDirty = (isDirty: boolean) => {
|
||||
const target = event.target.closest<HTMLAnchorElement>(anchorSelector);
|
||||
if (!target) return;
|
||||
|
||||
const destination = new URL(target.href, window.location.href);
|
||||
const current = new URL(window.location.href);
|
||||
const navigationKind = classifyGuardedNavigation(destination, current);
|
||||
if (navigationKind === "same-url") return;
|
||||
if (navigationKind === "same-document") {
|
||||
event.preventDefault();
|
||||
guardedUrl = destination.href;
|
||||
replaceGuardedSameDocumentUrl(destination, { key: guardStateKey, id: guardId });
|
||||
return;
|
||||
}
|
||||
|
||||
event.preventDefault();
|
||||
const destination = new URL(target.href, window.location.href);
|
||||
|
||||
openLeaveConfirmation(() => {
|
||||
if (destination.origin === window.location.origin) {
|
||||
@@ -659,7 +648,7 @@ const usePreventLeaveWithDirty = (isDirty: boolean) => {
|
||||
window.location.href === guardedUrl &&
|
||||
(window.history.state as Record<string, unknown> | null)?.[guardStateKey] === guardId
|
||||
) {
|
||||
removeGuardHistoryEntry(guardedUrl, guardStateKey);
|
||||
window.history.back();
|
||||
}
|
||||
};
|
||||
}, [isDirty]);
|
||||
|
||||
@@ -18,20 +18,6 @@ describe("resolveMobileItemPresentation", () => {
|
||||
});
|
||||
});
|
||||
|
||||
test("does not unmount apps that use the eager default", () => {
|
||||
expect(
|
||||
resolveMobileItemPresentation(
|
||||
{ kind: "app", height: 1 },
|
||||
{
|
||||
unmountWhenOffscreen: true,
|
||||
},
|
||||
),
|
||||
).toMatchObject({
|
||||
eager: true,
|
||||
unmountWhenOffscreen: false,
|
||||
});
|
||||
});
|
||||
|
||||
test("defaults widgets to full width without inventing a detail view", () => {
|
||||
expect(resolveMobileItemPresentation({ kind: "calendar", height: 8 }, undefined)).toMatchObject({
|
||||
width: 2,
|
||||
|
||||
@@ -8,7 +8,6 @@ export const resolveMobileItemPresentation = (
|
||||
const supportsCompactSummary = mobile?.supportsCompactSummary === true;
|
||||
const supportsDetails = mobile?.supportsDetailView === true;
|
||||
const usesGenericSummary = supportsDetails && !supportsCompactSummary;
|
||||
const eager = mobile?.eager ?? (item.kind === "app" || item.kind === "bookmarks");
|
||||
|
||||
return {
|
||||
width: mobile?.width ?? (item.kind === "app" ? 1 : 2),
|
||||
@@ -16,8 +15,8 @@ export const resolveMobileItemPresentation = (
|
||||
displayMode: supportsCompactSummary || usesGenericSummary ? ("mobileSummary" as const) : ("default" as const),
|
||||
supportsDetails,
|
||||
usesGenericSummary,
|
||||
eager,
|
||||
unmountWhenOffscreen: eager ? false : (mobile?.unmountWhenOffscreen ?? false),
|
||||
eager: mobile?.eager ?? (item.kind === "app" || item.kind === "bookmarks"),
|
||||
unmountWhenOffscreen: mobile?.eager === true ? false : (mobile?.unmountWhenOffscreen ?? false),
|
||||
};
|
||||
};
|
||||
|
||||
|
||||
@@ -12,7 +12,7 @@ describe("resolveMobileBoardViewport", () => {
|
||||
resolveMobileBoardViewport({
|
||||
deviceClass: "phone",
|
||||
matchesMobileWidth: false,
|
||||
hasMounted: true,
|
||||
hasResolvedClientWidth: true,
|
||||
}),
|
||||
).toEqual({
|
||||
deviceClass: "phone",
|
||||
@@ -29,7 +29,7 @@ describe("resolveMobileBoardViewport", () => {
|
||||
resolveMobileBoardViewport({
|
||||
deviceClass: "tablet",
|
||||
matchesMobileWidth,
|
||||
hasMounted: true,
|
||||
hasResolvedClientWidth: true,
|
||||
}).isMobile,
|
||||
).toBe(expectedIsMobile);
|
||||
});
|
||||
@@ -39,7 +39,7 @@ describe("resolveMobileBoardViewport", () => {
|
||||
resolveMobileBoardViewport({
|
||||
deviceClass: "tablet",
|
||||
matchesMobileWidth: false,
|
||||
hasMounted: false,
|
||||
hasResolvedClientWidth: false,
|
||||
}).isResolved,
|
||||
).toBe(false);
|
||||
});
|
||||
@@ -52,7 +52,7 @@ describe("resolveMobileBoardViewport", () => {
|
||||
resolveMobileBoardViewport({
|
||||
deviceClass: "desktop",
|
||||
matchesMobileWidth,
|
||||
hasMounted: true,
|
||||
hasResolvedClientWidth: true,
|
||||
}),
|
||||
).toMatchObject({
|
||||
isMobile: expectedIsMobile,
|
||||
@@ -73,9 +73,7 @@ describe("resolveIsAutomaticMobileBoard", () => {
|
||||
expect(resolveIsAutomaticMobileBoard({ isMobileViewport, enableAutomaticMobileLayout })).toBe(expected);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe("shouldShowMobileBoardViewportSkeleton", () => {
|
||||
test("does not show the tablet resolution skeleton in legacy mode", () => {
|
||||
expect(
|
||||
shouldShowMobileBoardViewportSkeleton({
|
||||
@@ -84,13 +82,4 @@ describe("shouldShowMobileBoardViewportSkeleton", () => {
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
test("shows a skeleton while an automatic mobile viewport is unresolved", () => {
|
||||
expect(
|
||||
shouldShowMobileBoardViewportSkeleton({
|
||||
enableAutomaticMobileLayout: true,
|
||||
isResolved: false,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -26,15 +26,15 @@ const MobileBoardViewportContext = createContext<MobileBoardViewport>({
|
||||
export const resolveMobileBoardViewport = ({
|
||||
deviceClass,
|
||||
matchesMobileWidth,
|
||||
hasMounted,
|
||||
hasResolvedClientWidth,
|
||||
}: {
|
||||
deviceClass: MobileBoardDeviceClass;
|
||||
matchesMobileWidth: boolean;
|
||||
hasMounted: boolean;
|
||||
hasResolvedClientWidth: boolean;
|
||||
}): MobileBoardViewport => ({
|
||||
deviceClass,
|
||||
isMobile: deviceClass === "phone" || matchesMobileWidth,
|
||||
isResolved: deviceClass !== "tablet" || hasMounted,
|
||||
isResolved: deviceClass !== "tablet" || hasResolvedClientWidth,
|
||||
});
|
||||
|
||||
const MobileBoardViewportSkeleton = () => (
|
||||
@@ -73,19 +73,19 @@ export const MobileBoardViewportProvider = ({
|
||||
children,
|
||||
}: PropsWithChildren<{ initialDeviceClass: MobileBoardDeviceClass }>) => {
|
||||
const { enableAutomaticMobileLayout } = useSettings();
|
||||
const [hasMounted, setHasMounted] = useState(false);
|
||||
const [hasResolvedClientWidth, setHasResolvedClientWidth] = useState(false);
|
||||
const matchesMobileWidth = useMediaQuery(mobileBoardMediaQuery, initialDeviceClass === "phone", {
|
||||
getInitialValueInEffect: true,
|
||||
});
|
||||
|
||||
useEffect(() => {
|
||||
setHasMounted(true);
|
||||
setHasResolvedClientWidth(true);
|
||||
}, []);
|
||||
|
||||
const viewport = resolveMobileBoardViewport({
|
||||
deviceClass: initialDeviceClass,
|
||||
matchesMobileWidth,
|
||||
hasMounted,
|
||||
hasResolvedClientWidth,
|
||||
});
|
||||
|
||||
return (
|
||||
|
||||
Reference in new issue
Block a user