Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Basic CRUD for userData structure, setup ListContext #3198

Merged
merged 3 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions editor.planx.uk/src/@planx/components/List/Public/Context.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import React, { createContext, ReactNode, useContext, useState } from "react";

import { generateNewItem, Schema, UserData } from "../model";

interface ListContextValue {
schema: Schema;
activeIndex: number | undefined;
userData: UserData;
addNewItem: () => void;
saveItem: (index: number, updatedItem: UserData[0]) => void;
removeItem: (index: number) => void;
editItem: (index: number) => void;
cancelEditItem: () => void;
}

interface ListProviderProps {
children: ReactNode;
schema: Schema;
}

const ListContext = createContext<ListContextValue | undefined>(undefined);

export const ListProvider: React.FC<ListProviderProps> = ({
children,
schema,
}) => {
const [activeIndex, setActiveIndex] = useState<number | undefined>(0);
const [userData, setUserData] = useState<UserData>(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not 100% sure how this will interact with a Fromik instance, so this may get updated in the next PR to either be just be Fromik state, or this can be generated by Formik.

I've intentionally avoided this in the current PR to keep thing separate and as small as possible, even if it may mean a little more "churn".

schema.min === 0 ? [] : [generateNewItem(schema)],
);

const addNewItem = () => {
setUserData([...userData, generateNewItem(schema)]);
setActiveIndex((prev) => (prev === undefined ? 0 : prev + 1));
};

const saveItem = (index: number, updatedItem: UserData[0]) => {
setUserData((prev) =>
prev.map((item, i) => (i === index ? updatedItem : item)),
);
};

const editItem = (index: number) => setActiveIndex(index);

const removeItem = (index: number) => {
if (activeIndex && index < activeIndex) {
// If item is before currently active card, retain active card
setActiveIndex((prev) => (prev === undefined ? 0 : prev - 1));
} else if (index === activeIndex || index === 0) {
// If item is currently in Edit mode, exit Edit mode
cancelEditItem();
}

// Remove item from userData
setUserData((prev) => prev.filter((_, i) => i !== index));
};

const cancelEditItem = () => setActiveIndex(undefined);

return (
<ListContext.Provider
value={{
activeIndex,
userData,
addNewItem,
saveItem,
schema,
editItem,
removeItem,
cancelEditItem,
}}
>
{children}
</ListContext.Provider>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

);
};

export const useListContext = (): ListContextValue => {
const context = useContext(ListContext);
if (!context) {
throw new Error("useListContext must be used within a ListProvider");
}
return context;
};
268 changes: 265 additions & 3 deletions editor.planx.uk/src/@planx/components/List/Public/index.test.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { within } from "@testing-library/react";
import { cloneDeep, merge } from "lodash";
import React from "react";
import { axe, setup } from "testUtils";

Expand Down Expand Up @@ -99,9 +101,269 @@ describe("Navigating back", () => {
});

describe("Building a list", () => {
test.todo("Adding an item");
test.todo("Editing an item");
test.todo("Removing an item");
it("does not display a default item if the schema has no required minimum", () => {
const mockWithMinZero = merge(cloneDeep(mockProps), { schema: { min: 0 } });
const { queryByRole, getByRole } = setup(
<ListComponent {...mockWithMinZero} />,
);

// No card present
const activeListHeading = queryByRole("heading", {
level: 2,
name: "Animal 1",
});
expect(activeListHeading).toBeNull();

// Button is present allow additional items to be added
const addItemButton = getByRole("button", {
name: /Add a new animal type/,
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: introducing & using a stable data-testid attribute for the addItemButton & similar early might mean less tedious updates based on "name" later and more generic tests independent of schemas?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great shout actually - will make that change 👍

Copy link
Contributor

@RODO94 RODO94 May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: @jessicamcinchak what does "nit" mean?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shorthand for "nit picky" ! opinionated minor suggestions basically, almost always non-blocking to overall approval and readiness to merge 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RODO94 We talked about conventional commits for PR names as part of onboarding. Using nit/nitpick comes from conventional comments which we use here and there also 👍

expect(addItemButton).toBeInTheDocument();
expect(addItemButton).not.toBeDisabled();
});

it("displays a default item if the schema has a required minimum", () => {
const { getByRole, queryByLabelText } = setup(
<ListComponent {...mockProps} />,
);

// Card present...
const activeListHeading = getByRole("heading", {
level: 2,
name: "Animal 1",
});
expect(activeListHeading).toBeInTheDocument();

// ...with active fields
const inputField = queryByLabelText(/What's their name?/);
expect(inputField).toBeInTheDocument();
expect(inputField).not.toBeDisabled();
});

test("Adding an item", async () => {
const { getAllByRole, getByRole, user } = setup(
<ListComponent {...mockProps} />,
);

let cards = getAllByRole("heading", { level: 2 }).map((el) =>
el.closest("div"),
);
expect(cards).toHaveLength(1);

const addItemButton = getByRole("button", {
name: /Add a new animal type/,
});
await user.click(addItemButton);

// Item successfully added
cards = getAllByRole("heading", { level: 2 }).map((el) =>
el.closest("div"),
);
expect(cards).toHaveLength(2);

// Old item is inactive
const [firstCard, secondCard] = cards;
expect(firstCard).not.toBeNull();
expect(
within(firstCard!).queryByLabelText(/What's their name?/),
).toBeNull();

// New item is active
expect(secondCard).not.toBeNull();
expect(
within(secondCard!).getByLabelText(/What's their name?/),
).toBeInTheDocument();
});

test("Editing an item", async () => {
// Setup three cards
const { getAllByRole, getByRole, user, findByLabelText } = setup(
<ListComponent {...mockProps} />,
);

const addItemButton = getByRole("button", {
name: /Add a new animal type/,
});
await user.click(addItemButton);
await user.click(addItemButton);

let cards = getAllByRole("heading", { level: 2 }).map((el) =>
el.closest("div"),
);
expect(cards).toHaveLength(3);

let [firstCard, secondCard, thirdCard] = cards;

// Final card is currently active
expect(thirdCard).not.toBeNull();
expect(
within(thirdCard!).getByLabelText(/What's their name?/),
).toBeInTheDocument();

// Hitting "cancel" takes us out of Edit mode
const thirdCardCancelButton = within(thirdCard!).getByRole("button", {
name: /Cancel/,
});
await user.click(thirdCardCancelButton);

cards = getAllByRole("heading", { level: 2 }).map((el) =>
el.closest("div"),
);
[firstCard, secondCard, thirdCard] = cards;

// No cards currently active
expect(
within(firstCard!).queryByLabelText(/What's their name?/),
).toBeNull();
expect(
within(secondCard!).queryByLabelText(/What's their name?/),
).toBeNull();
expect(
within(thirdCard!).queryByLabelText(/What's their name?/),
).toBeNull();

// All card in view only / summary mode
expect(within(firstCard!).getByText(/What's their name?/)).toBeVisible();
expect(within(secondCard!).getByText(/What's their name?/)).toBeVisible();
expect(within(thirdCard!).getByText(/What's their name?/)).toBeVisible();

// Hit "Edit" on second card
const secondCardEditButton = within(secondCard!).getByRole("button", {
name: /Edit/,
});
await user.click(secondCardEditButton);

cards = getAllByRole("heading", { level: 2 }).map((el) =>
el.closest("div"),
);
[firstCard, secondCard, thirdCard] = cards;

// Second card now editable
expect(
within(secondCard!).getByLabelText(/What's their name?/),
).toBeInTheDocument();
});

test("Removing an item when all cards are inactive", async () => {
// Setup three cards
const { getAllByRole, getByRole, user, getByLabelText, queryAllByRole } =
setup(<ListComponent {...mockProps} />);

const addItemButton = getByRole("button", {
name: /Add a new animal type/,
});
await user.click(addItemButton);
await user.click(addItemButton);

let cards = getAllByRole("heading", { level: 2 }).map((el) =>
el.closest("div"),
);
expect(cards).toHaveLength(3);

let [firstCard, secondCard, thirdCard] = cards;

const thirdCardCancelButton = within(thirdCard!).getByRole("button", {
name: /Cancel/,
});
await user.click(thirdCardCancelButton);

[firstCard, secondCard, thirdCard] = getAllByRole("heading", {
level: 2,
}).map((el) => el.closest("div"));

// Remove third card
const thirdCardRemoveButton = within(thirdCard!).getByRole("button", {
name: /Remove/,
});
await user.click(thirdCardRemoveButton);
cards = getAllByRole("heading", { level: 2 }).map((el) =>
el.closest("div"),
);
expect(cards).toHaveLength(2);

[firstCard, secondCard] = getAllByRole("heading", { level: 2 }).map((el) =>
el.closest("div"),
);

// Previous items remain inactive
expect(
within(firstCard!).queryByLabelText(/What's their name?/),
).toBeNull();
expect(
within(secondCard!).queryByLabelText(/What's their name?/),
).toBeNull();

// Remove second card
const secondCardRemoveButton = within(secondCard!).getByRole("button", {
name: /Remove/,
});
await user.click(secondCardRemoveButton);
cards = getAllByRole("heading", { level: 2 }).map((el) =>
el.closest("div"),
);
expect(cards).toHaveLength(1);

[firstCard] = getAllByRole("heading", { level: 2 }).map((el) =>
el.closest("div"),
);

// Previous items remain inactive
expect(
within(firstCard!).queryByLabelText(/What's their name?/),
).toBeNull();

// Remove first card
const firstCardRemoveButton = within(firstCard!).getByRole("button", {
name: /Remove/,
});
await user.click(firstCardRemoveButton);
cards = queryAllByRole("heading", { level: 2 }).map((el) =>
el.closest("div"),
);
expect(cards).toHaveLength(0);

// Add item back
await user.click(addItemButton);

// This is now editable and active
const newFirstCardInput = getByLabelText(/What's their name?/);
expect(newFirstCardInput).toBeInTheDocument();
});

test("Removing an item when another card is active", async () => {
// Setup two cards
const { getAllByRole, getByRole, user, getByLabelText, queryAllByRole } =
setup(<ListComponent {...mockProps} />);

const addItemButton = getByRole("button", {
name: /Add a new animal type/,
});
await user.click(addItemButton);

const [firstCard, secondCard] = getAllByRole("heading", { level: 2 }).map(
(el) => el.closest("div"),
);

// Second card is active
expect(
within(secondCard!).getByLabelText(/What's their name?/),
).toBeInTheDocument();

// Remove first
const firstCardRemoveButton = within(firstCard!).getByRole("button", {
name: /Remove/,
});
await user.click(firstCardRemoveButton);
const cards = getAllByRole("heading", { level: 2 }).map((el) =>
el.closest("div"),
);
expect(cards).toHaveLength(1);

// First card is active
expect(
within(cards[0]!).getByLabelText(/What's their name?/),
).toBeInTheDocument();
});
});

describe("Form validation and error handling", () => {
Expand Down
Loading
Loading