Skip to content

Commit

Permalink
fix: ensure layer tree consistently shows selected item
Browse files Browse the repository at this point in the history
The expansion state of the LayerTree component is based on the pathData data, which is set by components mounting and calling `registerPath`. However, the pathData object contained selectors with hardcoded indexes for where each item was in their zone. If the item moved, the pathData wouldn't be updated. This was noticed as the LayerTree would sometimes avoid expanding the parent area (i.e. Columns in the demo application) of a selected item because it couldn't find it.

This would not occur for components in the default-zone.

This fix changes the underlying implementation of pathData, going from storing the selectors to the zoneCompound IDs (i.e. `[MyComponent:zone1,MyComponent:zone2]`), and calulating the selectors dynamically when necessary through a new `use-breadcrumbs` hook.

This hook isn't required for the LayerTree component, which relies on `is-child-of-zone`. `is-child-of-zone` can now easily determine whether or not the item is a child by checking for the presence of an area in the pathData.

This change also adds test coverage.
  • Loading branch information
chrisvxd committed Oct 17, 2023
1 parent 2a12826 commit 6a9145c
Show file tree
Hide file tree
Showing 10 changed files with 273 additions and 61 deletions.
22 changes: 9 additions & 13 deletions packages/core/components/DropZone/context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,7 @@ import { rootDroppableId } from "../../lib/root-droppable-id";
import { useDebounce } from "use-debounce";
import { getZoneId } from "../../lib/get-zone-id";

type PathData = Record<
string,
{ selector: ItemSelector | null; label: string }[]
>;
export type PathData = Record<string, { path: string[]; label: string }>;

export type DropZoneContext = {
data: Data;
Expand Down Expand Up @@ -129,17 +126,17 @@ export const DropZoneProvider = ({
const [area] = getZoneId(selector.zone);

setPathData((latestPathData = {}) => {
const pathData = latestPathData[area] || [];
const parentPathData = latestPathData[area] || { path: [] };

return {
...latestPathData,
[item.props.id]: [
...pathData,
{
selector,
label: item.type as string,
},
],
[item.props.id]: {
path: [
...parentPathData.path,
...(selector.zone ? [selector.zone] : []),
],
label: item.type as string,
},
};
});
},
Expand All @@ -164,7 +161,6 @@ export const DropZoneProvider = ({
activeZones,
registerPath,
pathData,

...value,
}}
>
Expand Down
8 changes: 7 additions & 1 deletion packages/core/components/Puck/context.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { createContext, useContext } from "react";
import { AppData, AppState } from "../../types/Config";
import { PuckAction } from "../../reducer";
import { getItem } from "../../lib/get-item";

export const defaultAppData: AppData = {
data: { content: [], root: { title: "" } },
Expand All @@ -25,9 +26,14 @@ export const AppProvider = appContext.Provider;
export const useAppContext = () => {
const mainContext = useContext(appContext);

const selectedItem = mainContext.appData.state.itemSelector
? getItem(mainContext.appData.state.itemSelector, mainContext.appData.data)
: undefined;

return {
...mainContext,
// Helper
// Helpers
selectedItem,
setState: (state: Partial<AppState>, recordHistory?: boolean) => {
return mainContext.dispatch({
type: "setState",
Expand Down
15 changes: 1 addition & 14 deletions packages/core/components/Puck/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -265,16 +265,6 @@ export function Puck({
>
<dropZoneContext.Consumer>
{(ctx) => {
let path =
ctx?.pathData && selectedItem
? ctx?.pathData[selectedItem?.props.id]
: undefined;

if (path) {
path = [{ label: "Page", selector: null }, ...path];
path = path.slice(path.length - 2, path.length - 1);
}

return (
<div
style={{
Expand Down Expand Up @@ -515,10 +505,7 @@ export function Puck({
<FieldWrapper data={data}>
<SidebarSection
noPadding
breadcrumbs={path}
breadcrumbClick={(breadcrumb) =>
setItemSelector(breadcrumb.selector)
}
showBreadcrumbs
title={selectedItem ? selectedItem.type : "Page"}
>
{Object.keys(fields).map((fieldName) => {
Expand Down
40 changes: 22 additions & 18 deletions packages/core/components/SidebarSection/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,42 +3,46 @@ import styles from "./styles.module.css";
import getClassNameFactory from "../../lib/get-class-name-factory";
import { Heading } from "../Heading";
import { ChevronRight } from "react-feather";
import { ItemSelector } from "../../lib/get-item";
import { useBreadcrumbs } from "../../lib/use-breadcrumbs";
import { useAppContext } from "../Puck/context";

const getClassName = getClassNameFactory("SidebarSection", styles);

type Breadcrumb = { label: string; selector: ItemSelector | null };

export const SidebarSection = ({
children,
title,
background,
breadcrumbs = [],
breadcrumbClick,
showBreadcrumbs,
noPadding,
}: {
children: ReactNode;
title: ReactNode;
background?: string;
breadcrumbs?: Breadcrumb[];
breadcrumbClick?: (breadcrumb: Breadcrumb) => void;
showBreadcrumbs?: boolean;
noPadding?: boolean;
}) => {
const { setState } = useAppContext();
const breadcrumbs = useBreadcrumbs(1);

return (
<div className={getClassName({ noPadding })} style={{ background }}>
<div className={getClassName("title")}>
<div className={getClassName("breadcrumbs")}>
{breadcrumbs.map((breadcrumb, i) => (
<div key={i} className={getClassName("breadcrumb")}>
<div
className={getClassName("breadcrumbLabel")}
onClick={() => breadcrumbClick && breadcrumbClick(breadcrumb)}
>
{breadcrumb.label}
</div>
<ChevronRight size={16} />
</div>
))}
{showBreadcrumbs
? breadcrumbs.map((breadcrumb, i) => (
<div key={i} className={getClassName("breadcrumb")}>
<div
className={getClassName("breadcrumbLabel")}
onClick={() =>
setState({ itemSelector: breadcrumb.selector })
}
>
{breadcrumb.label}
</div>
<ChevronRight size={16} />
</div>
))
: null}
<div className={getClassName("heading")}>
<Heading rank={2} size="xs">
{title}
Expand Down
52 changes: 52 additions & 0 deletions packages/core/lib/__tests__/is-child-of-zone.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { DropZoneContext } from "../../components/DropZone/context";
import { Config, Data } from "../../types/Config";
import { isChildOfZone } from "../is-child-of-zone";

const item1 = { type: "MyComponent", props: { id: "MyComponent-1" } };
const item2 = { type: "MyComponent", props: { id: "MyComponent-2" } };
const item3 = { type: "MyComponent", props: { id: "MyComponent-3" } };

const data: Data = {
root: { title: "" },
content: [item1],
zones: {
"MyComponent-1:zone": [item2],
"MyComponent-2:zone": [item3],
},
};

const config: Config = {
components: {
Comp: {
defaultProps: { prop: "example" },
render: () => <div />,
},
},
};

const dropzoneContext: DropZoneContext = {
data,
config,
pathData: {
"MyComponent-1": { path: [], label: "MyComponent" },
"MyComponent-2": { path: ["MyComponent-1:zone"], label: "MyComponent" },
"MyComponent-3": {
path: ["MyComponent-1:zone", "MyComponent-2:zone"],
label: "MyComponent",
},
},
};

describe("is-child-of-zone", () => {
it("should return true when item is child of zone for first item", () => {
expect(isChildOfZone(item1, item2, dropzoneContext)).toBe(true);
});

it("should return true when item is child of zone for second item", () => {
expect(isChildOfZone(item2, item3, dropzoneContext)).toBe(true);
});

it("should return false when item is not child of zone", () => {
expect(isChildOfZone(item2, item1, dropzoneContext)).toBe(false);
});
});
67 changes: 67 additions & 0 deletions packages/core/lib/__tests__/use-breadcrumbs.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { DropZoneContext } from "../../components/DropZone/context";
import { Config, Data } from "../../types/Config";
import { Breadcrumb, convertPathDataToBreadcrumbs } from "../use-breadcrumbs";

const item1 = { type: "MyComponent", props: { id: "MyComponent-1" } };
const item2 = { type: "MyComponent", props: { id: "MyComponent-2" } };
const item3 = { type: "MyComponent", props: { id: "MyComponent-3" } };

const data: Data = {
root: { title: "" },
content: [item1],
zones: {
"MyComponent-1:zone": [item2],
"MyComponent-2:zone": [item3],
},
};

const config: Config = {
components: {
Comp: {
defaultProps: { prop: "example" },
render: () => <div />,
},
},
};

const dropzoneContext: DropZoneContext = {
data,
config,
pathData: {
"MyComponent-1": { path: [], label: "MyComponent" },
"MyComponent-2": { path: ["MyComponent-1:zone"], label: "MyComponent" },
"MyComponent-3": {
path: ["MyComponent-1:zone", "MyComponent-2:zone"],
label: "MyComponent",
},
},
};

describe("use-breadcrumbs", () => {
describe("convert-path-data-to-breadcrumbs", () => {
it("should convert path data to breadcrumbs", () => {
expect(
convertPathDataToBreadcrumbs(item3, dropzoneContext.pathData, data)
).toMatchInlineSnapshot(`
[
{
"label": "MyComponent",
"selector": {
"index": 0,
"zone": "default-zone",
},
"zoneCompound": "MyComponent-1:zone",
},
{
"label": "MyComponent",
"selector": {
"index": 0,
"zone": "MyComponent-1:zone",
},
"zoneCompound": "MyComponent-2:zone",
},
]
`);
});
});
});
4 changes: 3 additions & 1 deletion packages/core/lib/get-zone-id.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { rootDroppableId } from "./root-droppable-id";

export const getZoneId = (zoneCompound?: string) => {
if (!zoneCompound) {
return [];
Expand All @@ -7,5 +9,5 @@ export const getZoneId = (zoneCompound?: string) => {
return zoneCompound.split(":");
}

return ["root", zoneCompound];
return [rootDroppableId, zoneCompound];
};
11 changes: 4 additions & 7 deletions packages/core/lib/is-child-of-zone.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { DropZoneContext } from "../components/DropZone/context";
import { Content } from "../types/Config";
import { getItem } from "./get-item";
import { getZoneId } from "./get-zone-id";

export const isChildOfZone = (
item: Content[0],
Expand All @@ -10,14 +11,10 @@ export const isChildOfZone = (
const { data, pathData = {} } = ctx || {};

return maybeChild && data
? !!pathData[maybeChild.props.id]?.find((path) => {
if (path.selector) {
const pathItem = getItem(path.selector!, data);
? !!pathData[maybeChild.props.id]?.path.find((zoneCompound) => {
const [area] = getZoneId(zoneCompound);

return pathItem?.props.id === item.props.id;
}

return false;
return area === item.props.id;
})
: false;
};
Loading

0 comments on commit 6a9145c

Please sign in to comment.