diff --git a/.changeset/quiet-ravens-watch.md b/.changeset/quiet-ravens-watch.md new file mode 100644 index 000000000..9c9321fc4 --- /dev/null +++ b/.changeset/quiet-ravens-watch.md @@ -0,0 +1,6 @@ +--- +"@tailor-platform/app-shell": patch +--- + +Use the current viewport for the sidebar's first client-only render instead of initially treating mobile and tablet widths as desktop. +Existing breakpoints and the desktop fallback during server rendering and initial hydration remain unchanged. diff --git a/packages/core/src/components/sidebar/sidebar-layout.test.tsx b/packages/core/src/components/sidebar/sidebar-layout.test.tsx index 6c3c8e1bb..6a9bc0e21 100644 --- a/packages/core/src/components/sidebar/sidebar-layout.test.tsx +++ b/packages/core/src/components/sidebar/sidebar-layout.test.tsx @@ -33,12 +33,9 @@ const createMinimalModules = () => [ }), ]; -/** - * Stub window.innerWidth so useIsMobile() returns false (desktop mode). - * This is needed because happy-dom defaults to innerWidth=0 which triggers mobile mode. - */ +/** Use a width outside both mobile and tablet ranges for desktop assertions. */ const stubDesktopViewport = () => { - vi.spyOn(window, "innerWidth", "get").mockReturnValue(1024); + vi.spyOn(window, "innerWidth", "get").mockReturnValue(1200); window.dispatchEvent(new Event("resize")); }; @@ -64,6 +61,17 @@ describe("SidebarLayout", () => { }); }); + it("renders the tablet icon rail at 1024px", async () => { + vi.spyOn(window, "innerWidth", "get").mockReturnValue(1024); + renderSidebarLayout(); + + await waitFor(() => { + const sidebar = document.querySelector('[data-slot="sidebar"]'); + expect(sidebar?.getAttribute("data-state")).toBe("collapsed"); + expect(sidebar?.getAttribute("data-collapsible")).toBe("icon"); + }); + }); + it("renders sidebar in collapsed state when defaultOpen is false", async () => { stubDesktopViewport(); renderSidebarLayout({ defaultOpen: false }); diff --git a/packages/core/src/hooks/use-mobile.ts b/packages/core/src/hooks/use-mobile.ts index 502fd3239..6d3dd5378 100644 --- a/packages/core/src/hooks/use-mobile.ts +++ b/packages/core/src/hooks/use-mobile.ts @@ -2,18 +2,14 @@ import * as React from "react"; const MOBILE_BREAKPOINT = 768; -export function useIsMobile() { - const [isMobile, setIsMobile] = React.useState(undefined); - - React.useEffect(() => { - const mql = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`); - const onChange = () => { - setIsMobile(window.innerWidth < MOBILE_BREAKPOINT); - }; - mql.addEventListener("change", onChange); - setIsMobile(window.innerWidth < MOBILE_BREAKPOINT); - return () => mql.removeEventListener("change", onChange); - }, []); +const subscribe = (onChange: () => void) => { + const media = window.matchMedia(`(max-width: ${MOBILE_BREAKPOINT - 1}px)`); + media.addEventListener("change", onChange); + return () => media.removeEventListener("change", onChange); +}; +const getSnapshot = () => window.innerWidth < MOBILE_BREAKPOINT; +const getServerSnapshot = () => false; - return !!isMobile; +export function useIsMobile() { + return React.useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot); } diff --git a/packages/core/src/hooks/use-responsive.test.tsx b/packages/core/src/hooks/use-responsive.test.tsx new file mode 100644 index 000000000..f970a3c15 --- /dev/null +++ b/packages/core/src/hooks/use-responsive.test.tsx @@ -0,0 +1,93 @@ +import { act, cleanup, render, renderHook } from "@testing-library/react"; +import { renderToString } from "react-dom/server"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useIsMobile } from "./use-mobile"; +import { useIsTablet } from "./use-tablet"; + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); + vi.unstubAllGlobals(); +}); + +function ViewportProbe({ onRender }: { onRender?: (mobile: boolean, tablet: boolean) => void }) { + const mobile = useIsMobile(); + const tablet = useIsTablet(); + onRender?.(mobile, tablet); + return {`${mobile}/${tablet}`}; +} + +describe("responsive viewport hooks", () => { + it.each([ + [767, true, false], + [768, false, true], + [1199, false, true], + [1200, false, false], + ] as const)("reads %ipx on the first client render", (width, mobile, tablet) => { + vi.spyOn(window, "innerWidth", "get").mockReturnValue(width); + const onRender = vi.fn(); + + render(); + + expect(onRender).toHaveBeenNthCalledWith(1, mobile, tablet); + }); + + it("updates on media changes without resubscribing on rerender and unsubscribes on unmount", () => { + const width = vi.spyOn(window, "innerWidth", "get").mockReturnValue(1200); + const media = window.matchMedia("(min-width: 0px)"); + const addListener = vi.spyOn(media, "addEventListener"); + const removeListener = vi.spyOn(media, "removeEventListener"); + const matchMedia = vi.spyOn(window, "matchMedia").mockReturnValue(media); + const { result, rerender, unmount } = renderHook(() => ({ + mobile: useIsMobile(), + tablet: useIsTablet(), + })); + + expect(result.current).toEqual({ mobile: false, tablet: false }); + expect(matchMedia.mock.calls).toEqual([ + ["(max-width: 767px)"], + ["(min-width: 768px) and (max-width: 1199px)"], + ]); + + for (const [nextWidth, mobile, tablet] of [ + [767, true, false], + [768, false, true], + [1199, false, true], + [1200, false, false], + ] as const) { + act(() => { + width.mockReturnValue(nextWidth); + media.dispatchEvent(new Event("change")); + }); + expect(result.current).toEqual({ mobile, tablet }); + } + + rerender(); + expect(addListener).toHaveBeenCalledTimes(2); + unmount(); + expect(removeListener.mock.calls).toEqual(addListener.mock.calls); + }); + + it("returns false during SSR without accessing window", () => { + vi.stubGlobal("window", undefined); + + expect(renderToString()).toBe("false/false"); + }); + + it.each([ + [375, true, false], + [1024, false, true], + ] as const)("hydrates the server fallback before reading %ipx", (width, mobile, tablet) => { + vi.spyOn(window, "innerWidth", "get").mockReturnValue(width); + const container = document.createElement("div"); + container.innerHTML = renderToString(); + const onRender = vi.fn(); + const errors = vi.spyOn(console, "error"); + + render(, { container, hydrate: true }); + + expect(onRender).toHaveBeenNthCalledWith(1, false, false); + expect(onRender).toHaveBeenLastCalledWith(mobile, tablet); + expect(errors).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/src/hooks/use-tablet.ts b/packages/core/src/hooks/use-tablet.ts index 42748ef81..2e64183c3 100644 --- a/packages/core/src/hooks/use-tablet.ts +++ b/packages/core/src/hooks/use-tablet.ts @@ -3,24 +3,17 @@ import * as React from "react"; const TABLET_MIN_BREAKPOINT = 768; const TABLET_MAX_BREAKPOINT = 1199; -export function useIsTablet() { - const [isTablet, setIsTablet] = React.useState(undefined); - - React.useEffect(() => { - const mql = window.matchMedia( - `(min-width: ${TABLET_MIN_BREAKPOINT}px) and (max-width: ${TABLET_MAX_BREAKPOINT}px)`, - ); - const onChange = () => { - setIsTablet( - window.innerWidth >= TABLET_MIN_BREAKPOINT && window.innerWidth <= TABLET_MAX_BREAKPOINT, - ); - }; - mql.addEventListener("change", onChange); - setIsTablet( - window.innerWidth >= TABLET_MIN_BREAKPOINT && window.innerWidth <= TABLET_MAX_BREAKPOINT, - ); - return () => mql.removeEventListener("change", onChange); - }, []); +const subscribe = (onChange: () => void) => { + const media = window.matchMedia( + `(min-width: ${TABLET_MIN_BREAKPOINT}px) and (max-width: ${TABLET_MAX_BREAKPOINT}px)`, + ); + media.addEventListener("change", onChange); + return () => media.removeEventListener("change", onChange); +}; +const getSnapshot = () => + window.innerWidth >= TABLET_MIN_BREAKPOINT && window.innerWidth <= TABLET_MAX_BREAKPOINT; +const getServerSnapshot = () => false; - return !!isTablet; +export function useIsTablet() { + return React.useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot); }