diff --git a/cypress/e2e/with-users/settings/dhcp.spec.ts b/cypress/e2e/with-users/settings/dhcp.spec.ts new file mode 100644 index 0000000000..e96e0de302 --- /dev/null +++ b/cypress/e2e/with-users/settings/dhcp.spec.ts @@ -0,0 +1,31 @@ +import { generateMAASURL, generateName } from "../../utils"; + +context("Settings - DHCP Snippets", () => { + beforeEach(() => { + cy.login(); + cy.addMachine(); + cy.visit(generateMAASURL("/settings/dhcp/add")); + }); + + it("can add a DHCP snippet to a machine", () => { + const snippetName = generateName("dhcp-snippet"); + cy.get("[data-testid='section-header-title']").contains("Settings"); + cy.findByLabelText("Snippet name").type(snippetName); + cy.findByLabelText("Type").select("Machine"); + cy.findByRole("button", { name: /Choose machine/ }).click(); + // ensure the data has loaded + cy.findByRole("grid").should("have.attr", "aria-busy", "false"); + cy.get("tbody").within(() => { + cy.findAllByRole("row").first().click(); + }); + cy.findByLabelText("DHCP snippet").type("ddns-update-style none;"); + cy.findByRole("button", { name: "Save snippet" }).click(); + // expect to be redirected to the list page + cy.findByLabelText("Search DHCP snippets").type(snippetName); + cy.findByRole("grid").within(() => { + cy.findByText(snippetName).should("be.visible"); + cy.findByRole("button", { name: /Delete/ }).click(); + cy.get("[data-testid='action-confirm']").click(); + }); + }); +}); diff --git a/cypress/support/commands.ts b/cypress/support/commands.ts index d66f25e4c0..3462777a94 100644 --- a/cypress/support/commands.ts +++ b/cypress/support/commands.ts @@ -1,5 +1,7 @@ import "@testing-library/cypress/add-commands"; import type { Result } from "axe-core"; +import { nanoid } from "nanoid"; +import { generateMAASURL, generateMac } from "../e2e/utils"; import type { A11yPageContext } from "./e2e"; Cypress.Commands.add("login", (options) => { @@ -35,6 +37,17 @@ Cypress.Commands.add("loginNonAdmin", () => { }); }); +Cypress.Commands.add("addMachine", (hostname = `cypress-${nanoid()}`) => { + cy.visit(generateMAASURL("/machines")); + cy.get("[data-testid='add-hardware-dropdown'] button").click(); + cy.get(".p-contextual-menu__link").contains("Machine").click(); + cy.get("input[name='hostname']").type(hostname); + cy.get("input[name='pxe_mac']").type(generateMac()); + cy.get("select[name='power_type']").select("manual").blur(); + cy.get("button[type='submit']").click(); + cy.get(`[data-testid='message']:contains(${hostname} added successfully.)`); +}); + function logViolations(violations: Result[], pageContext: A11yPageContext) { const divider = "\n====================================================================================================\n"; diff --git a/cypress/support/e2e.ts b/cypress/support/e2e.ts index 110e95a47f..c564f763b1 100644 --- a/cypress/support/e2e.ts +++ b/cypress/support/e2e.ts @@ -9,6 +9,7 @@ export type A11yPageContext = { url?: string; title?: string }; declare global { namespace Cypress { interface Chainable { + addMachine(hostname?: string): void; login(options?: { username?: string; password?: string; diff --git a/src/app/base/components/DebounceSearchBox/DebounceSearchBox.tsx b/src/app/base/components/DebounceSearchBox/DebounceSearchBox.tsx index e91e9be9cf..8e855efa45 100644 --- a/src/app/base/components/DebounceSearchBox/DebounceSearchBox.tsx +++ b/src/app/base/components/DebounceSearchBox/DebounceSearchBox.tsx @@ -9,7 +9,7 @@ type Props = { onDebounced: (debouncedText: string) => void; searchText: string; setSearchText: (searchText: string) => void; -} & Omit; +} & Omit; export const DEFAULT_DEBOUNCE_INTERVAL = 500; @@ -22,6 +22,7 @@ const DebounceSearchBox = ({ onDebounced, searchText, setSearchText, + ...props }: Props): JSX.Element => { const intervalRef = useRef(null); const [debouncing, setDebouncing] = useState(false); @@ -38,6 +39,7 @@ const DebounceSearchBox = ({ return (
{ setDebouncing(true); diff --git a/src/app/base/components/DhcpFormFields/DhcpFormFields.test.tsx b/src/app/base/components/DhcpFormFields/DhcpFormFields.test.tsx index b64d0765ba..648310f24e 100644 --- a/src/app/base/components/DhcpFormFields/DhcpFormFields.test.tsx +++ b/src/app/base/components/DhcpFormFields/DhcpFormFields.test.tsx @@ -1,4 +1,5 @@ -import { render, screen } from "@testing-library/react"; +import reduxToolkit from "@reduxjs/toolkit"; +import { render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { Provider } from "react-redux"; import { MemoryRouter } from "react-router-dom"; @@ -16,17 +17,20 @@ import { dhcpSnippetState as dhcpSnippetStateFactory, machine as machineFactory, machineState as machineStateFactory, + machineStateList as machineStateListFactory, + machineStateListGroup as machineStateListGroupFactory, subnet as subnetFactory, subnetState as subnetStateFactory, rootState as rootStateFactory, } from "testing/factories"; const mockStore = configureStore(); - +const machines = [machineFactory()]; describe("DhcpFormFields", () => { let state: RootState; beforeEach(() => { + jest.spyOn(reduxToolkit, "nanoid").mockReturnValue("123456"); state = rootStateFactory({ controller: controllerStateFactory({ loaded: true }), device: deviceStateFactory({ loaded: true }), @@ -49,11 +53,19 @@ describe("DhcpFormFields", () => { loaded: true, }), machine: machineStateFactory({ - items: [ - machineFactory({ - fqdn: "node2.maas", + items: machines, + lists: { + "123456": machineStateListFactory({ + loading: false, + loaded: true, + groups: [ + machineStateListGroupFactory({ + items: [machines[0].system_id], + name: "Deployed", + }), + ], }), - ], + }, loaded: true, }), subnet: subnetStateFactory({ @@ -113,7 +125,7 @@ describe("DhcpFormFields", () => { screen.getByRole("alert", { name: Labels.LoadingData }) ).toBeInTheDocument(); expect( - screen.queryByRole("combobox", { name: Labels.Entity }) + screen.queryByRole("combobox", { name: Labels.AppliesTo }) ).not.toBeInTheDocument(); }); @@ -136,7 +148,7 @@ describe("DhcpFormFields", () => { screen.queryByRole("alert", { name: Labels.LoadingData }) ).not.toBeInTheDocument(); expect( - screen.getByRole("combobox", { name: Labels.Entity }) + screen.getByRole("combobox", { name: Labels.AppliesTo }) ).toBeInTheDocument(); }); @@ -154,15 +166,31 @@ describe("DhcpFormFields", () => { ); // Set an initial type. const typeSelect = screen.getByRole("combobox", { name: Labels.Type }); - await userEvent.selectOptions(typeSelect, "machine"); - + await userEvent.selectOptions(typeSelect, "subnet"); + await userEvent.selectOptions( + screen.getByRole("combobox", { + name: Labels.AppliesTo, + }), + "test.local" + ); // Select a machine. Value should get set. - const entitySelect = screen.getByRole("combobox", { name: Labels.Entity }); - await userEvent.selectOptions(entitySelect, machine.system_id); - expect(entitySelect).toHaveValue(machine.system_id); - + await userEvent.selectOptions(typeSelect, "machine"); + await userEvent.click( + screen.getByRole("button", { name: /Choose machine/ }) + ); + await waitFor(() => + expect(screen.getByRole("grid")).toHaveAttribute("aria-busy", "false") + ); + within(screen.getByRole("grid")).getByText(machine.hostname).click(); + expect( + screen.getByRole("button", { name: new RegExp(machine.hostname, "i") }) + ).toHaveAccessibleDescription(Labels.AppliesTo); // Change the type. The select value should be cleared. await userEvent.selectOptions(typeSelect, "subnet"); - expect(entitySelect).toHaveValue(""); + expect( + screen.getByRole("combobox", { + name: Labels.AppliesTo, + }) + ).toHaveValue(""); }); }); diff --git a/src/app/base/components/DhcpFormFields/DhcpFormFields.tsx b/src/app/base/components/DhcpFormFields/DhcpFormFields.tsx index e141c69b2f..5796d10ace 100644 --- a/src/app/base/components/DhcpFormFields/DhcpFormFields.tsx +++ b/src/app/base/components/DhcpFormFields/DhcpFormFields.tsx @@ -7,6 +7,8 @@ import { import { useFormikContext } from "formik"; import { useSelector } from "react-redux"; +import MachineSelect from "./MachineSelect/MachineSelect"; + import type { DHCPFormValues } from "app/base/components/DhcpForm/types"; import FormikField from "app/base/components/FormikField"; import controllerSelectors from "app/store/controller/selectors"; @@ -54,7 +56,7 @@ export enum Labels { Description = "Description", Disabled = "This snippet is disabled and will not be used by MAAS.", Enabled = "Enabled", - Entity = "Applies to", + AppliesTo = "Applies to", LoadingData = "Loading DHCP snippet data", Name = "Snippet name", Type = "Type", @@ -73,12 +75,8 @@ export const DhcpFormFields = ({ editing }: Props): JSX.Element => { const controllerLoaded = useSelector(controllerSelectors.loaded); const deviceLoading = useSelector(deviceSelectors.loading); const deviceLoaded = useSelector(deviceSelectors.loaded); - const machineLoading = useSelector(machineSelectors.loading); - const machineLoaded = useSelector(machineSelectors.loaded); - const isLoading = - subnetLoading || controllerLoading || deviceLoading || machineLoading; - const hasLoaded = - subnetLoaded && controllerLoaded && deviceLoaded && machineLoaded; + const isLoading = subnetLoading || controllerLoading || deviceLoading; + const hasLoaded = subnetLoaded && controllerLoaded && deviceLoaded; const { enabled, type } = formikProps.values; let models: ModelType[] | null; switch (type) { @@ -97,7 +95,6 @@ export const DhcpFormFields = ({ editing }: Props): JSX.Element => { default: models = null; } - return ( <> {editing && !enabled && ( @@ -134,21 +131,21 @@ export const DhcpFormFields = ({ editing }: Props): JSX.Element => { { value: "device", label: "Device" }, ]} /> - {type && + {type === "machine" ? ( + + ) : ( + type && (isLoading || !hasLoaded ? ( ) : ( - ))} + )) + )} { + renderWithMockStore( + + + + ); + + expect(screen.queryByRole("listbox")).not.toBeInTheDocument(); + await userEvent.click( + screen.getByRole("button", { name: new RegExp(Labels.ChooseMachine, "i") }) + ); + expect(screen.getByRole("listbox")).toBeInTheDocument(); +}); + +it("sets focus on the input field on open", async () => { + renderWithMockStore( + + + + ); + + await userEvent.click( + screen.getByRole("button", { name: new RegExp(Labels.ChooseMachine, "i") }) + ); + expect( + screen.getByPlaceholderText("Search by hostname, system ID or tags") + ).toHaveFocus(); +}); diff --git a/src/app/base/components/DhcpFormFields/MachineSelect/MachineSelect.tsx b/src/app/base/components/DhcpFormFields/MachineSelect/MachineSelect.tsx new file mode 100644 index 0000000000..bda59cbcd5 --- /dev/null +++ b/src/app/base/components/DhcpFormFields/MachineSelect/MachineSelect.tsx @@ -0,0 +1,100 @@ +import type { HTMLProps } from "react"; +import { useEffect, useState } from "react"; + +import { useId, useOnEscapePressed } from "@canonical/react-components"; +import Field from "@canonical/react-components/dist/components/Field"; +import className from "classnames"; +import { useFormikContext } from "formik"; +import { useDispatch } from "react-redux"; + +import SelectButton from "../../SelectButton"; + +import MachineSelectBox from "./MachineSelectBox/MachineSelectBox"; + +import OutsideClickHandler from "app/base/components/OutsideClickHandler"; +import { usePreviousPersistent } from "app/base/hooks"; +import type { FetchFilters, Machine } from "app/store/machine/types"; +import { useFetchMachine } from "app/store/machine/utils/hooks"; +import { actions as tagActions } from "app/store/tag"; + +export enum Labels { + AppliesTo = "Applies to", + Loading = "Loading...", + ChooseMachine = "Choose machine", +} + +export type Props = { + label?: React.ReactNode; + defaultOption?: string; + filters?: FetchFilters; + displayError?: boolean; + name: string; + value?: HTMLProps["value"]; +}; + +export const MachineSelect = ({ + name, + filters, + label = Labels.AppliesTo, + defaultOption = Labels.ChooseMachine, + value, + ...props +}: Props): JSX.Element => { + const { setFieldValue } = useFormikContext(); + const dispatch = useDispatch(); + const [isOpen, setIsOpen] = useState(false); + const labelId = useId(); + const selectId = useId(); + const handleSelect = (machine: Machine | null) => { + setIsOpen(false); + setFieldValue(name, machine?.system_id || null); + }; + useOnEscapePressed(() => setIsOpen(false)); + const { machine } = useFetchMachine(value as string); + const previousMachine = usePreviousPersistent(machine); + const selectedMachine = machine || previousMachine; + + useEffect(() => { + dispatch(tagActions.fetch()); + }, [dispatch]); + + const buttonLabel = selectedMachine?.hostname || defaultOption; + + return ( +
+ {/* TODO: update once Field allows a custom label id + https://github.com/canonical/react-components/issues/820 */} + {label}} {...props}> + setIsOpen(false)}> + { + setIsOpen(!isOpen); + if (!isOpen) { + setFieldValue(name, "", false); + } + }} + > + {buttonLabel} + +
+ {isOpen ? ( + + ) : null} +
+ + +
+ ); +}; + +export default MachineSelect; diff --git a/src/app/base/components/DhcpFormFields/MachineSelect/MachineSelectBox/MachineSelectBox.test.tsx b/src/app/base/components/DhcpFormFields/MachineSelect/MachineSelectBox/MachineSelectBox.test.tsx new file mode 100644 index 0000000000..a50cdb0863 --- /dev/null +++ b/src/app/base/components/DhcpFormFields/MachineSelect/MachineSelectBox/MachineSelectBox.test.tsx @@ -0,0 +1,98 @@ +import reduxToolkit from "@reduxjs/toolkit"; +import { screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import configureStore from "redux-mock-store"; + +import MachineSelectBox from "./MachineSelectBox"; + +import { DEFAULT_DEBOUNCE_INTERVAL } from "app/base/components/DebounceSearchBox/DebounceSearchBox"; +import { actions as machineActions } from "app/store/machine"; +import type { RootState } from "app/store/root/types"; +import { rootState as rootStateFactory } from "testing/factories"; +import { renderWithMockStore } from "testing/utils"; + +const mockStore = configureStore(); + +beforeEach(() => { + jest + .spyOn(reduxToolkit, "nanoid") + .mockReturnValueOnce("mocked-nanoid-1") + .mockReturnValueOnce("mocked-nanoid-2"); + jest.useFakeTimers(); +}); +afterEach(() => { + jest.restoreAllMocks(); + jest.useRealTimers(); +}); + +it("displays a listbox and a search input field", async () => { + renderWithMockStore(); + + expect(screen.getByRole("listbox")).toBeInTheDocument(); +}); + +it("fetches machines on mount", async () => { + const state = rootStateFactory(); + const store = mockStore(state); + renderWithMockStore(, { + store, + }); + + expect(screen.getByRole("listbox")).toBeInTheDocument(); + expect(screen.getByRole("listbox")).toBeInTheDocument(); + const expectedAction = machineActions.fetch("mocked-nanoid-1", { + filter: { free_text: "" }, + group_collapsed: undefined, + group_key: null, + page_number: 1, + page_size: 15, + sort_direction: null, + sort_key: null, + }); + expect( + store.getActions().find((action) => action.type === expectedAction.type) + ).toStrictEqual(expectedAction); +}); + +it("requests machines filtered by the free text input value", async () => { + const state = rootStateFactory(); + const store = mockStore(state); + renderWithMockStore(, { + store, + }); + + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + await user.type(screen.getByRole("combobox"), "test-machine"); + const expectedInitialAction = machineActions.fetch("mocked-nanoid-1", { + filter: { free_text: "" }, + group_collapsed: undefined, + group_key: null, + page_number: 1, + page_size: 15, + sort_direction: null, + sort_key: null, + }); + const expectedAction = machineActions.fetch("mocked-nanoid-2", { + filter: { free_text: "test-machine" }, + group_collapsed: undefined, + group_key: null, + page_number: 1, + page_size: 15, + sort_direction: null, + sort_key: null, + }); + await waitFor(() => { + jest.advanceTimersByTime(DEFAULT_DEBOUNCE_INTERVAL); + }); + await waitFor(() => + expect( + store.getActions().filter((action) => action.type === expectedAction.type) + .length + ).toEqual(2) + ); + const machineFetchActions = store + .getActions() + .filter((action) => action.type === expectedAction.type); + expect(machineFetchActions[0]).toStrictEqual(expectedInitialAction); + expect(machineFetchActions[1]).toStrictEqual(expectedAction); +}); diff --git a/src/app/base/components/DhcpFormFields/MachineSelect/MachineSelectBox/MachineSelectBox.tsx b/src/app/base/components/DhcpFormFields/MachineSelect/MachineSelectBox/MachineSelectBox.tsx new file mode 100644 index 0000000000..780723310b --- /dev/null +++ b/src/app/base/components/DhcpFormFields/MachineSelect/MachineSelectBox/MachineSelectBox.tsx @@ -0,0 +1,67 @@ +import { useState } from "react"; + +import DebounceSearchBox from "app/base/components/DebounceSearchBox"; +import { MachineSelectTable } from "app/base/components/MachineSelectTable/MachineSelectTable"; +import MachineListPagination from "app/machines/views/MachineList/MachineListTable/MachineListPagination"; +import type { FetchFilters, Machine } from "app/store/machine/types"; +import { FilterGroupKey } from "app/store/machine/types"; +import { useFetchMachines } from "app/store/machine/utils/hooks"; + +const MachineSelectBox = ({ + onSelect, + pageSize = 15, + filters, +}: { + pageSize?: number; + onSelect: (machine: Machine | null) => void; + filters?: FetchFilters; +}): JSX.Element => { + const [searchText, setSearchText] = useState(""); + const [debouncedText, setDebouncedText] = useState(""); + const [currentPage, setCurrentPage] = useState(1); + const { machines, machineCount, loading } = useFetchMachines({ + filters: { + [FilterGroupKey.FreeText]: debouncedText, + ...(filters ? filters : {}), + }, + pagination: { + currentPage, + pageSize, + setCurrentPage, + }, + }); + return ( +
+ +
+ { + onSelect(machine); + }} + searchText={searchText} + setSearchText={setSearchText} + /> + +
+
+ ); +}; + +export default MachineSelectBox; diff --git a/src/app/base/components/DhcpFormFields/MachineSelect/MachineSelectBox/index.ts b/src/app/base/components/DhcpFormFields/MachineSelect/MachineSelectBox/index.ts new file mode 100644 index 0000000000..3845ff87d4 --- /dev/null +++ b/src/app/base/components/DhcpFormFields/MachineSelect/MachineSelectBox/index.ts @@ -0,0 +1 @@ +export { default } from "./MachineSelectBox"; diff --git a/src/app/base/components/DhcpFormFields/MachineSelect/_index.scss b/src/app/base/components/DhcpFormFields/MachineSelect/_index.scss new file mode 100644 index 0000000000..72a0eec75d --- /dev/null +++ b/src/app/base/components/DhcpFormFields/MachineSelect/_index.scss @@ -0,0 +1,19 @@ +@mixin MachineSelect() { + .machine-select { + margin-bottom: $spv--large; + } + .machine-select-box-wrapper { + transform: translate3d(0, -0.5rem, 0); + @include vf-animation(#{transform, opacity}); + opacity: 0; + &--is-open { + opacity: 1; + transform: translate3d(0, 0, 0); + } + } + .machine-select-box { + @extend %vf-has-box-shadow; + @extend %vf-is-bordered; + padding: $spv--medium $sph--large 0 $sph--large; + } +} diff --git a/src/app/base/components/DhcpFormFields/MachineSelect/index.ts b/src/app/base/components/DhcpFormFields/MachineSelect/index.ts new file mode 100644 index 0000000000..944f4c8242 --- /dev/null +++ b/src/app/base/components/DhcpFormFields/MachineSelect/index.ts @@ -0,0 +1 @@ +export { default } from "./MachineSelect"; diff --git a/src/app/base/components/MachineSelectTable/MachineSelectTable.test.tsx b/src/app/base/components/MachineSelectTable/MachineSelectTable.test.tsx index 572fc586db..170d5498c1 100644 --- a/src/app/base/components/MachineSelectTable/MachineSelectTable.test.tsx +++ b/src/app/base/components/MachineSelectTable/MachineSelectTable.test.tsx @@ -46,7 +46,7 @@ describe("MachineSelectTable", () => { }); }); - it("shows a spinner while data is loading", () => { + it("shows a loading skeleton while data is loading", () => { state.machine.loading = true; renderWithMockStore( { />, { state } ); - expect(screen.getByText(Label.Loading)).toBeInTheDocument(); + expect(screen.getByRole("grid")).toHaveAttribute("aria-busy", "true"); }); it("highlights the substring that matches the search text", async () => { @@ -110,4 +110,25 @@ describe("MachineSelectTable", () => { }); expect(within(ownerCols[0]).getByText("tagA")).toBeInTheDocument(); }); + + it("can select machine by pressing Enter key", async () => { + const onMachineClick = jest.fn(); + const machine = machines[0]; + renderWithMockStore( + , + { state } + ); + screen + .getByRole("row", { + name: machine.hostname, + }) + .focus(); + await userEvent.keyboard("{enter}"); + expect(onMachineClick).toHaveBeenCalledWith(machine); + }); }); diff --git a/src/app/base/components/MachineSelectTable/MachineSelectTable.tsx b/src/app/base/components/MachineSelectTable/MachineSelectTable.tsx index a71199f4e8..8fbd61e74e 100644 --- a/src/app/base/components/MachineSelectTable/MachineSelectTable.tsx +++ b/src/app/base/components/MachineSelectTable/MachineSelectTable.tsx @@ -1,9 +1,13 @@ +import type { KeyboardEvent } from "react"; import { useEffect } from "react"; -import { MainTable, Spinner } from "@canonical/react-components"; +import { MainTable } from "@canonical/react-components"; import { highlightSubString } from "@canonical/react-components/dist/utils"; import { useDispatch, useSelector } from "react-redux"; +import Placeholder from "../Placeholder"; +import VisuallyHidden from "../VisuallyHidden"; + import DoubleRow from "app/base/components/DoubleRow"; import machineSelectors from "app/store/machine/selectors"; import type { Machine } from "app/store/machine/types"; @@ -22,6 +26,7 @@ type Props = { machines: Machine[]; onMachineClick: (machine: Machine | null) => void; searchText: string; + machinesLoading?: boolean; setSearchText: (searchText: string) => void; }; @@ -69,12 +74,45 @@ const generateRows = ( ], "data-testid": "machine-select-row", onClick: () => onRowClick(machine), + onKeyPress: (e: KeyboardEvent) => { + if (e.key === "Enter") { + e.preventDefault(); + onRowClick(machine); + } + }, tabIndex: 0, })); }; +const getSkeletonRows = () => + Array.from(Array(3)).map((_, i) => ({ + className: "machine-select-table__row", + columns: [ + { + content: ( + xxxxxxxxx.xxxx} + secondary={xxxxxxxxx.xxxx} + /> + ), + }, + { + content: ( + xxxxxxxxx.xxxx} + secondary={xxxxxxxxx.xxxx} + /> + ), + }, + ], + "data-testid": "machine-select-row", + tabIndex: -1, + key: i, + })); + export const MachineSelectTable = ({ machines, + machinesLoading, onMachineClick, searchText, setSearchText, @@ -87,40 +125,49 @@ export const MachineSelectTable = ({ dispatch(tagActions.fetch()); }, [dispatch]); - if (loadingMachines) { - return ; - } + const rows = generateRows( + machines, + searchText, + (machine) => { + setSearchText(machine.hostname); + onMachineClick(machine); + }, + tags + ); + + const skeletonRows = getSkeletonRows(); + return ( - -
{Label.Hostname}
-
system_id
- - ), - }, - { - content: ( - <> -
{Label.Owner}
-
Tags
- - ), - }, - ]} - rows={generateRows( - machines, - searchText, - (machine) => { - setSearchText(machine.hostname); - onMachineClick(machine); - }, - tags - )} - /> + <> + +
{Label.Hostname}
+
system_id
+ + ), + }, + { + content: ( + <> +
{Label.Owner}
+
Tags
+ + ), + }, + ]} + rows={machinesLoading ? skeletonRows : rows} + /> + +
{machinesLoading ? "loading" : null}
+ + ); }; diff --git a/src/app/base/components/OutsideClickHandler/OutsideClickHandler.test.tsx b/src/app/base/components/OutsideClickHandler/OutsideClickHandler.test.tsx new file mode 100644 index 0000000000..d104900ce4 --- /dev/null +++ b/src/app/base/components/OutsideClickHandler/OutsideClickHandler.test.tsx @@ -0,0 +1,20 @@ +import { screen, render } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; + +import OutsideClickHandler from "./OutsideClickHandler"; + +it("calls the onClick handler when clicking outside of the component", async () => { + const onClick = jest.fn(); + render( +
+
Outside
+ +
Inside
+ +
+ ); + await userEvent.click(screen.getByText("Inside")); + expect(onClick).not.toHaveBeenCalled(); + await userEvent.click(screen.getByText("Outside")); + expect(onClick).toHaveBeenCalled(); +}); diff --git a/src/app/base/components/OutsideClickHandler/OutsideClickHandler.tsx b/src/app/base/components/OutsideClickHandler/OutsideClickHandler.tsx new file mode 100644 index 0000000000..9c1864ec8f --- /dev/null +++ b/src/app/base/components/OutsideClickHandler/OutsideClickHandler.tsx @@ -0,0 +1,14 @@ +import { useClickOutside } from "@canonical/react-components"; + +const OutsideClickHandler = ({ + children, + onClick, +}: React.PropsWithChildren<{ + onClick: () => void; +}>): JSX.Element => { + const [ref] = useClickOutside(onClick); + + return
{children}
; +}; + +export default OutsideClickHandler; diff --git a/src/app/base/components/OutsideClickHandler/index.ts b/src/app/base/components/OutsideClickHandler/index.ts new file mode 100644 index 0000000000..a593e92f38 --- /dev/null +++ b/src/app/base/components/OutsideClickHandler/index.ts @@ -0,0 +1 @@ +export { default } from "./OutsideClickHandler"; diff --git a/src/app/base/components/SelectButton/SelectButton.test.tsx b/src/app/base/components/SelectButton/SelectButton.test.tsx new file mode 100644 index 0000000000..f0dc6a2726 --- /dev/null +++ b/src/app/base/components/SelectButton/SelectButton.test.tsx @@ -0,0 +1,9 @@ +import { render, screen } from "@testing-library/react"; + +import SelectButton from "./SelectButton"; + +it("displays a button", () => { + render(Test); + + expect(screen.getByRole("button", { name: "Test" })).toBeInTheDocument(); +}); diff --git a/src/app/base/components/SelectButton/SelectButton.tsx b/src/app/base/components/SelectButton/SelectButton.tsx new file mode 100644 index 0000000000..2183cf3c71 --- /dev/null +++ b/src/app/base/components/SelectButton/SelectButton.tsx @@ -0,0 +1,20 @@ +import type { ButtonProps } from "@canonical/react-components"; +import { Button, Icon } from "@canonical/react-components"; +import classNames from "classnames"; + +const SelectButton = ({ + children, + className, + ...props +}: ButtonProps): JSX.Element => ( + +); + +export default SelectButton; diff --git a/src/app/base/components/SelectButton/_index.scss b/src/app/base/components/SelectButton/_index.scss new file mode 100644 index 0000000000..fa6f8ce354 --- /dev/null +++ b/src/app/base/components/SelectButton/_index.scss @@ -0,0 +1,14 @@ +@mixin SelectButton { + .p-button--select { + display: flex; + width: 100%; + text-align: left; + justify-content: space-between; + align-items: center; + border-radius: 0; + padding-left: $sph--small; + } + .p-button--select:hover { + background-color: inherit; + } +} diff --git a/src/app/base/components/SelectButton/index.ts b/src/app/base/components/SelectButton/index.ts new file mode 100644 index 0000000000..1de9d83aa4 --- /dev/null +++ b/src/app/base/components/SelectButton/index.ts @@ -0,0 +1 @@ +export { default } from "./SelectButton"; diff --git a/src/app/base/hooks/base.test.tsx b/src/app/base/hooks/base.test.tsx index 179c391657..f6a27f338c 100644 --- a/src/app/base/hooks/base.test.tsx +++ b/src/app/base/hooks/base.test.tsx @@ -3,6 +3,7 @@ import { act, renderHook } from "@testing-library/react-hooks"; import { useCycled, useId, + usePreviousPersistent, useProcessing, useScrollOnRender, useScrollToTop, @@ -272,4 +273,26 @@ describe("hooks", () => { expect(scrollToSpy).toHaveBeenCalledTimes(1); }); }); + + describe("usePreviousPersistent", () => { + it("should return null on initial render", () => { + const { result } = renderHook(() => usePreviousPersistent({ a: "b" })); + + expect(result.current).toBeNull(); + }); + + it("persists previous values on re-render", () => { + const { rerender, result } = renderHook( + (state) => usePreviousPersistent(state), + { + initialProps: 1, + } + ); + + rerender(2); + expect(result.current).toEqual(1); + rerender(3); + expect(result.current).toEqual(2); + }); + }); }); diff --git a/src/app/base/hooks/base.ts b/src/app/base/hooks/base.ts index 3cdb62b877..10551b938a 100644 --- a/src/app/base/hooks/base.ts +++ b/src/app/base/hooks/base.ts @@ -175,3 +175,28 @@ export const useScrollToTop = (): void => { window.scrollTo(0, 0); }, [pathname]); }; + +/** + * Returns the previous value reference persisted across the render cycles + * @param value - value to persist across the render cycles + * @returns previous value + */ +export const usePreviousPersistent = ( + value: T +): T | null => { + const ref = useRef<{ value: T; prev: T | null }>({ + value: value, + prev: null, + }); + + const current = ref.current.value; + + if (value !== current) { + ref.current = { + value: value, + prev: current, + }; + } + + return ref.current.prev; +}; diff --git a/src/app/base/hooks/index.ts b/src/app/base/hooks/index.ts index f2c8117ed1..d34f14dc56 100644 --- a/src/app/base/hooks/index.ts +++ b/src/app/base/hooks/index.ts @@ -11,6 +11,7 @@ export { useScrollOnRender, useScrollToTop, useWindowTitle, + usePreviousPersistent, } from "./base"; export { useFormikFormDisabled, useFormikErrors } from "./forms"; export { useCompletedIntro, useCompletedUserIntro } from "./intro"; diff --git a/src/app/dashboard/views/DiscoveriesList/_index.scss b/src/app/dashboard/views/DiscoveriesList/_index.scss index 8699e539f9..ec043a26f1 100644 --- a/src/app/dashboard/views/DiscoveriesList/_index.scss +++ b/src/app/dashboard/views/DiscoveriesList/_index.scss @@ -1,5 +1,5 @@ @mixin DiscoveriesList { - .p-table--network-discoveries { + .p-table--network-discoveries > tbody > { th, td { &:nth-child(1) { diff --git a/src/app/dashboard/views/DiscoveryAddForm/DiscoveryAddFormFields/DiscoveryAddFormFields.tsx b/src/app/dashboard/views/DiscoveryAddForm/DiscoveryAddFormFields/DiscoveryAddFormFields.tsx index 9ebbbd5d17..5adacf1bef 100644 --- a/src/app/dashboard/views/DiscoveryAddForm/DiscoveryAddFormFields/DiscoveryAddFormFields.tsx +++ b/src/app/dashboard/views/DiscoveryAddForm/DiscoveryAddFormFields/DiscoveryAddFormFields.tsx @@ -6,6 +6,7 @@ import { Link } from "react-router-dom-v5-compat"; import type { DiscoveryAddValues } from "../types"; import { DeviceType } from "../types"; +import MachineSelect from "app/base/components/DhcpFormFields/MachineSelect"; import FormikField from "app/base/components/FormikField"; import IpAssignmentSelect from "app/base/components/IpAssignmentSelect"; import TooltipButton from "app/base/components/TooltipButton"; @@ -15,7 +16,6 @@ import type { Device } from "app/store/device/types"; import { DeviceMeta } from "app/store/device/types"; import type { Discovery } from "app/store/discovery/types"; import domainSelectors from "app/store/domain/selectors"; -import { useFetchMachines } from "app/store/machine/utils/hooks"; import type { RootState } from "app/store/root/types"; import subnetSelectors from "app/store/subnet/selectors"; import { getSubnetDisplay } from "app/store/subnet/utils"; @@ -29,6 +29,21 @@ type Props = { setDeviceType: (deviceType: DeviceType) => void; }; +export enum Label { + ChooseType = "Choose type", + ChooseDomain = "Choose domain", + Type = "Type", + Device = "Device", + DeviceName = "Device Name", + Interface = "Interface", + Parent = "Parent", + Hostname = "Hostname (optional)", + InterfaceName = "Interface name (optional)", + Domain = "Domain", + SelectDeviceName = "Select device name", + SelectParent = "Select parent (optional)", +} + const DiscoveryAddFormFields = ({ discovery, setDevice, @@ -36,10 +51,6 @@ const DiscoveryAddFormFields = ({ }: Props): JSX.Element | null => { const devices = useSelector(deviceSelectors.all); const domains = useSelector(domainSelectors.all); - const { machines } = useFetchMachines({ - filters: { status: FetchNodeStatus.DEPLOYED }, - }); - const subnet = useSelector((state: RootState) => subnetSelectors.getByCIDR(state, discovery.subnet_cidr) ); @@ -60,7 +71,7 @@ const DiscoveryAddFormFields = ({ ) => { setFieldValue("type", evt.target.value); @@ -69,24 +80,24 @@ const DiscoveryAddFormFields = ({ setDevice(null); }} options={[ - { label: "Choose type", value: "", disabled: true }, - { label: "Device", value: DeviceType.DEVICE }, - { label: "Interface", value: DeviceType.INTERFACE }, + { label: Label.ChooseType, value: "", disabled: true }, + { label: Label.Device, value: DeviceType.DEVICE }, + { label: Label.Interface, value: DeviceType.INTERFACE }, ]} required /> {isDevice ? ( ({ label: domain.name, value: domain.name, @@ -100,7 +111,7 @@ const DiscoveryAddFormFields = ({ component={Select} label={ <> - Device name{" "} + {Label.DeviceName}{" "} } @@ -110,7 +121,7 @@ const DiscoveryAddFormFields = ({ setDevice(evt.target.value as Device[DeviceMeta.PK]); }} options={[ - { label: "Select device name", value: "", disabled: true }, + { label: Label.SelectDeviceName, value: "", disabled: true }, ...devices.map((device) => ({ label: device.fqdn, value: device[DeviceMeta.PK], @@ -120,21 +131,16 @@ const DiscoveryAddFormFields = ({ /> ) : ( - Parent{" "} + {Label.Parent}{" "} } name="parent" - options={[ - { label: "Select parent (optional)", value: "" }, - ...machines.map((machine) => ({ - label: machine.fqdn, - value: machine.system_id, - })), - ]} /> )} diff --git a/src/app/machines/components/MachineHeaderForms/ActionFormWrapper/DeployForm/DeployFormFields/DeployFormFields.tsx b/src/app/machines/components/MachineHeaderForms/ActionFormWrapper/DeployForm/DeployFormFields/DeployFormFields.tsx index 02a2cfd265..9e8eea6295 100644 --- a/src/app/machines/components/MachineHeaderForms/ActionFormWrapper/DeployForm/DeployFormFields/DeployFormFields.tsx +++ b/src/app/machines/components/MachineHeaderForms/ActionFormWrapper/DeployForm/DeployFormFields/DeployFormFields.tsx @@ -100,11 +100,7 @@ export const DeployFormFields = (): JSX.Element => { clearVmHostOptions(); } }} - options={ - // This won't need to pass the empty array once this issue is fixed: - // https://github.com/canonical/react-components/issues/570 - osOptions || [] - } + options={osOptions} /> diff --git a/src/app/settings/views/Configuration/CommissioningFormFields/CommissioningFormFields.tsx b/src/app/settings/views/Configuration/CommissioningFormFields/CommissioningFormFields.tsx index b6dec827d2..1659245c13 100644 --- a/src/app/settings/views/Configuration/CommissioningFormFields/CommissioningFormFields.tsx +++ b/src/app/settings/views/Configuration/CommissioningFormFields/CommissioningFormFields.tsx @@ -57,11 +57,7 @@ const CommissioningFormFields = (): JSX.Element => { formikProps.setFieldValue("default_min_hwe_kernel", kernelValue); formikProps.setFieldTouched("default_min_hwe_kernel", true, true); }} - options={ - // This won't need to pass the empty array once this issue is fixed: - // https://github.com/canonical/react-components/issues/570 - distroSeriesOptions || [] - } + options={distroSeriesOptions} /> { help="When enabled, MAAS will use passive techniques (such as listening to ARP requests and mDNS advertisements) to observe networks attached to rack controllers. Active subnet mapping will also be available to be enabled on the configured subnets." label="Network discovery" name="network_discovery" - options={ - // This won't need to pass the empty array once this issue is fixed: - // https://github.com/canonical/react-components/issues/570 - networkDiscoveryOptions || [] - } + options={networkDiscoveryOptions} /> { help="When enabled, each rack will scan subnets enabled for active mapping. This helps ensure discovery information is accurate and complete." label="Active subnet mapping interval" name="active_discovery_interval" - options={ - // This won't need to pass the empty array once this issue is fixed: - // https://github.com/canonical/react-components/issues/570 - discoveryIntervalOptions || [] - } + options={discoveryIntervalOptions} /> ); diff --git a/src/app/store/machine/utils/hooks.ts b/src/app/store/machine/utils/hooks.ts index 0437c4616f..047db53f8d 100644 --- a/src/app/store/machine/utils/hooks.ts +++ b/src/app/store/machine/utils/hooks.ts @@ -222,7 +222,11 @@ export const useFetchMachine = ( } }, [dispatch, id, callId, previousCallId]); - return { machine, loading, loaded }; + return { + machine, + loading, + loaded, + }; }; /** diff --git a/src/scss/index.scss b/src/scss/index.scss index 48fbac73d8..8694c92ae8 100644 --- a/src/scss/index.scss +++ b/src/scss/index.scss @@ -97,6 +97,7 @@ @import "~app/base/components/Popover"; @import "~app/base/components/Section"; @import "~app/base/components/SectionHeader"; +@import "~app/base/components/SelectButton"; @import "~app/base/components/SSHKeyList"; @import "~app/base/components/Stepper"; @import "~app/base/components/TableMenu"; @@ -129,6 +130,7 @@ @include Popover; @include Section; @include SectionHeader; +@include SelectButton; @include SSHKeyList; @include Stepper; @include TableMenu; @@ -162,6 +164,7 @@ @import "~app/kvm/components/CoreResources"; @import "~app/kvm/components/CPUColumn/CPUPopover"; @import "~app/kvm/components/KVMDetailsHeader"; +@import "~app/base/components/DhcpFormFields/MachineSelect"; @import "~app/kvm/components/KVMHeaderForms/ComposeForm/InterfacesTable"; @import "~app/kvm/components/KVMHeaderForms/ComposeForm/InterfacesTable/SubnetSelect"; @import "~app/kvm/components/KVMHeaderForms/ComposeForm/StorageTable"; @@ -201,6 +204,7 @@ @include LXDHostToolbar; @include LxdKVMHostTable; @include LXDVMsSummaryCard; +@include MachineSelect; @include NumaResources; @include NumaResourcesCard; @include RAMPopover;