Skip to content

Commit

Permalink
fix(layout): Do not unmount children when swapping to non-fixed appba…
Browse files Browse the repository at this point in the history
…r mini layouts

Closes #1207
  • Loading branch information
mlaursen committed Jul 27, 2021
1 parent b889a9e commit 64103c8
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 36 deletions.
3 changes: 2 additions & 1 deletion packages/layout/src/LayoutChildren.tsx
Expand Up @@ -72,12 +72,13 @@ export interface LayoutChildrenProps<
* the `LayoutMain` and mini `LayoutNavigation` components.
*
* Note: This additional `<div>` will only be rendered if:
* - the current layout is one of the `mini` types
* - at least one of the provided layout types are `mini`
* - the layout is not using a fixed app bar
* - the `miniNav` prop has not been defined
* - `treeProps` have been provided
*
* @remarks \@since 2.8.3
* @remarks \@since 2.9.1 This will render if any provided layout type is `mini`.
*/
miniWrapperProps?: PropsWithRef<
HTMLAttributes<HTMLDivElement>,
Expand Down
29 changes: 26 additions & 3 deletions packages/layout/src/LayoutProvider.tsx
Expand Up @@ -17,7 +17,12 @@ import {
DEFAULT_TABLET_LAYOUT,
} from "./constants";
import { LayoutConfiguration, SupportedWideLayout } from "./types";
import { getLayoutType, isPersistentLayout, isToggleableLayout } from "./utils";
import {
getLayoutType,
isMiniLayout,
isPersistentLayout,
isToggleableLayout,
} from "./utils";

/**
* @internal
Expand Down Expand Up @@ -68,6 +73,15 @@ export interface LayoutContext {
* @remarks \@since 2.8.3
*/
fixedAppBar: boolean;

/**
* Boolean if one of the layout types are mini. This is mostly used internally
* to prevent the `<main>` element from unmounting (and losing state) for
* non-fixed app bar layouts.
*
* @remarks \@since 2.9.1
*/
isMiniable: boolean;
}

const context = createContext<LayoutContext>({
Expand All @@ -77,6 +91,7 @@ const context = createContext<LayoutContext>({
showNav: notInitialized("showNav"),
hideNav: notInitialized("hideNav"),
fixedAppBar: true,
isMiniable: false,
});

/**
Expand Down Expand Up @@ -143,6 +158,13 @@ export function LayoutProvider({
desktopLayout,
largeDesktopLayout,
});
const isMiniable = [
phoneLayout,
tabletLayout,
landscapeTabletLayout,
desktopLayout,
largeDesktopLayout,
].some((layout) => !!layout && isMiniLayout(layout));

const isPersistent = isPersistentLayout(layout);

Expand Down Expand Up @@ -171,16 +193,17 @@ export function LayoutProvider({
}
}, [layout]);

const value = useMemo(
const value = useMemo<LayoutContext>(
() => ({
baseId,
layout,
visible,
showNav,
hideNav,
fixedAppBar,
isMiniable,
}),
[baseId, layout, visible, showNav, hideNav, fixedAppBar]
[baseId, layout, visible, showNav, hideNav, fixedAppBar, isMiniable]
);

return <Provider value={value}>{children}</Provider>;
Expand Down
8 changes: 4 additions & 4 deletions packages/layout/src/MiniLayoutWrapper.tsx
Expand Up @@ -90,8 +90,8 @@ export function MiniLayoutWrapper({
containerProps,
...props
}: MiniLayoutWrapperProps): ReactElement {
const { fixedAppBar } = useLayoutConfig();
if (!mini || !treeProps || typeof miniNav !== "undefined") {
const { fixedAppBar, isMiniable } = useLayoutConfig();
if ((!mini && !isMiniable) || !treeProps || typeof miniNav !== "undefined") {
return (
<>
{miniNav}
Expand Down Expand Up @@ -136,9 +136,9 @@ export function MiniLayoutWrapper({
return (
<div
{...containerProps}
className={cn(!miniHidden && styles(), containerProps?.className)}
className={cn(!miniHidden && mini && styles(), containerProps?.className)}
>
{miniNavigation}
{mini && miniNavigation}
{children}
</div>
);
Expand Down
134 changes: 106 additions & 28 deletions packages/layout/src/__tests__/Layout.tsx
@@ -1,11 +1,13 @@
import React, { ReactElement } from "react";
import React, { ReactElement, useState } from "react";
import { createPortal } from "react-dom";
import {
Link,
BrowserRouter,
useLocation,
Switch,
Route,
} from "react-router-dom";
import { mocked } from "ts-jest/utils";
import {
RenderOptions,
fireEvent,
Expand All @@ -24,10 +26,12 @@ import {
SecuritySVGIcon,
SnoozeSVGIcon,
} from "@react-md/material-icons";
import { Checkbox } from "@react-md/form";
import { AppBarAction } from "@react-md/app-bar";
import {
DEFAULT_DESKTOP_MIN_WIDTH,
DEFAULT_DESKTOP_LARGE_MIN_WIDTH,
AppSizeListener,
AppSizeContext,
DEFAULT_APP_SIZE,
} from "@react-md/utils";

import { Configuration } from "../Configuration";
Expand All @@ -36,6 +40,41 @@ import { useLayoutConfig } from "../LayoutProvider";
import { useLayoutNavigation } from "../useLayoutNavigation";
import { LayoutNavigationTree } from "../types";

jest.mock("@react-md/utils", () => ({
...jest.requireActual("@react-md/utils"),
AppSizeListener: jest.fn(),
}));

mocked(AppSizeListener).mockImplementation(
({ children, defaultSize = DEFAULT_APP_SIZE }) => {
const [value, setValue] = useState({ ...defaultSize, __initialized: true });
return (
<AppSizeContext.Provider value={value}>
{children}
{createPortal(
// portal so it doesn't affect existing snapshots
<button
type="button"
onClick={() =>
setValue({
isPhone: true,
isTablet: false,
isDesktop: false,
isLandscape: false,
isLargeDesktop: false,
__initialized: true,
})
}
>
Mobile
</button>,
document.body
)}
</AppSizeContext.Provider>
);
}
);

const render = (ui: ReactElement, options?: RenderOptions) =>
baseRender(ui, {
...options,
Expand All @@ -51,6 +90,9 @@ const getById = (id: string) => {
return el;
};

const getMiniNav = () => getById("layout-mini-nav-container");
const getNav = () => getById("layout-nav-container");

const MULTIPLE_ROUTES_NAV_ITEMS: LayoutNavigationTree = {
"/": {
to: "/",
Expand Down Expand Up @@ -115,22 +157,6 @@ const MULTIPLE_ROUTES_NAV_ITEMS: LayoutNavigationTree = {
},
};

const matchMedia = jest.spyOn(window, "matchMedia");

const mockDesktop = () =>
matchMedia.mockImplementation((query) => ({
media: query,
matches:
query.includes(`${DEFAULT_DESKTOP_MIN_WIDTH}`) &&
query.includes(`${DEFAULT_DESKTOP_LARGE_MIN_WIDTH}`),
onchange: () => {},
addListener: () => {},
removeListener: () => {},
addEventListener: () => {},
removeEventListener: () => {},
dispatchEvent: () => false,
}));

describe("Layout", () => {
it("should render a typical layout with an AppBar, <main> element, and a skip to main content link with no additional props with ids", () => {
render(<Layout />);
Expand All @@ -149,8 +175,6 @@ describe("Layout", () => {
const clipped: LayoutProps = { desktopLayout: "clipped" };
const fullHeight: LayoutProps = { desktopLayout: "full-height" };

// just always use desktop config since it allows all options
mockDesktop();
const { rerender } = render(<Layout {...temporary} />);
expect(getById("layout-nav-toggle")).toBeInTheDocument();

Expand Down Expand Up @@ -336,10 +360,6 @@ describe("Layout", () => {
);
}

beforeAll(() => {
mockDesktop();
});

it("should default the toggleable layout visibility correctly", () => {
let { getByRole, unmount } = render(<Test />);
expect(() => getByRole("navigation")).toThrow();
Expand Down Expand Up @@ -452,9 +472,6 @@ describe("Layout", () => {
);
}

const getMiniNav = () => getById("layout-mini-nav-container");
const getNav = () => getById("layout-nav-container");

const { getByRole, queryAllByRole } = render(<Test />);
expect(getNav).toThrow();
expect(getMiniNav).not.toThrow();
Expand Down Expand Up @@ -621,4 +638,65 @@ describe("Layout", () => {
expect(route4).not.toHaveClass("rmd-tree-item__content--selected");
});
});

it("should maintain state while switching between static and mini layouts", () => {
const navItems: LayoutNavigationTree = {
"/": {
href: "/",
itemId: "/",
parentId: null,
children: "Home",
},
};

function App(): ReactElement {
const [checked, setChecked] = useState(false);

return (
<Checkbox
id="main-checkbox"
name="checkbox"
checked={checked}
label="Checkbox"
onChange={(event) => setChecked(event.currentTarget.checked)}
/>
);
}

function Test(): ReactElement {
const [selectedId, setSelectedId] = useState("/");
return (
<Layout
title="Non-fixed AppBar Layout Title"
navHeaderTitle="Another Title"
phoneLayout="temporary-mini"
tabletLayout="toggleable-mini"
landscapeTabletLayout="full-height"
desktopLayout="full-height"
appBarProps={{ fixed: false }}
treeProps={{
...useLayoutNavigation(navItems, selectedId),
onItemSelect: setSelectedId,
}}
>
<App />
</Layout>
);
}

const { getByRole } = render(<Test />);
expect(getNav).not.toThrow();
expect(getMiniNav).toThrow();
const checkbox = getByRole("checkbox", { name: "Checkbox" });
expect(checkbox).not.toBeChecked();

fireEvent.click(checkbox);
expect(checkbox).toBeChecked();

fireEvent.click(getByRole("button", { name: "Mobile" }));
expect(getNav).toThrow();
expect(getMiniNav).not.toThrow();
expect(checkbox).toBeInTheDocument();
expect(checkbox).toBeChecked();
});
});

0 comments on commit 64103c8

Please sign in to comment.