diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes index d88251cb5f..d6ea8b56b7 100644 --- a/web/package/agama-web-ui.changes +++ b/web/package/agama-web-ui.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Wed Apr 16 06:09:58 UTC 2025 - José Iván López González + +- Shorten storage device names. +- Move LVM logical volumes to partitions, if possible. +- Add MenuButton component (gh#agama-project/agama#2241). + ------------------------------------------------------------------- Tue Apr 15 08:51:27 UTC 2025 - Ancor Gonzalez Sosa diff --git a/web/src/assets/styles/index.scss b/web/src/assets/styles/index.scss index 28695c1d9b..82de4a1f73 100644 --- a/web/src/assets/styles/index.scss +++ b/web/src/assets/styles/index.scss @@ -171,24 +171,31 @@ } } -.agm-inline-menu-toggle { +.agm-inline-toggle { + border-radius: 0; inline-size: fit-content; - flex-direction: row-reverse; align-items: center; - padding: 0 var(--pf-t--global--spacer--control--vertical--compact); - background: var(--agm-t--color--fog); + padding: var(--pf-t--global--spacer--control--vertical--compact); - &:hover { + &:hover, + &.pf-m-expanded { background: #c0efde; // var(--agm-t--action--background--color--hover); + border-radius: var(--pf-t--global--border--radius--small); + + &::before { + border-radius: var(--pf-t--global--border--radius--small); + } } - &.pf-m-expanded { - background: var(--agm-t--color--jungle); + &::before { + border: 0; + border-radius: 0; + border-bottom: 1px solid var(--pf-t--global--border--color--default); } - .pf-v6-c-menu-toggle__icon svg { - block-size: var(--pf-t--global--icon--size--lg); - inline-size: var(--pf-t--global--icon--size--lg); + &:focus-visible::before { + color: red; + border-bottom: 0; } } @@ -355,6 +362,10 @@ label.pf-m-disabled + .pf-v6-c-check__description { color: var(--pf-v6-c-check__label--disabled--Color); } +.pf-v6-c-menu__list-item.pf-m-danger.pf-m-disabled { + --pf-v6-c-menu__item--Color: var(--pf-v6-c-menu__item--m-disabled--Color); +} + // Do not change the default cursor for labels forms because it is confusing // // See: diff --git a/web/src/components/core/MenuButton.test.tsx b/web/src/components/core/MenuButton.test.tsx new file mode 100644 index 0000000000..093ae7ac52 --- /dev/null +++ b/web/src/components/core/MenuButton.test.tsx @@ -0,0 +1,167 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { screen, within } from "@testing-library/react"; +import { plainRender } from "~/test-utils"; +import MenuButton, { MenuButtonItem } from "~/components/core/MenuButton"; + +it("renders a button", () => { + plainRender({"test"}); + const button = screen.getByRole("button", { name: "test" }); + expect(button).toBeInTheDocument(); +}); + +it("opens a menu on click", async () => { + const { user } = plainRender( + {"test"}, + ); + + const button = screen.getByRole("button", { name: "test" }); + const menu = screen.queryByRole("menu", { name: "test menu" }); + expect(menu).not.toBeInTheDocument(); + await user.click(button); + screen.getByRole("menu", { name: "test menu" }); +}); + +it("renders all the given menu items", async () => { + const { user } = plainRender( + {"item 1"}, + {"item 2"}, + {"item 3"}, + ]} + > + test + , + ); + + const button = screen.getByRole("button", { name: "test" }); + await user.click(button); + const menu = screen.getByRole("menu"); + within(menu).getByRole("menuitem", { name: "item 1" }); + within(menu).getByRole("menuitem", { name: "item 2" }); + within(menu).getByRole("menuitem", { name: "item 3" }); +}); + +it("allows passing props to the toggle", () => { + plainRender( + {"item 1"}, + {"item 2"}, + ]} + > + test + , + ); + + const button = screen.getByRole("button", { name: "test" }); + expect(button).toHaveClass("inline-toggle"); +}); + +it("allows to drill in", async () => { + const { user } = plainRender( + {"item 1-1"}, + {"item 1-2"}, + ]} + > + item 1 + , + ]} + > + test + , + ); + const button = screen.getByRole("button", { name: "test" }); + await user.click(button); + const menu = screen.getByRole("menu", { name: "test menu" }); + const item1 = within(menu).getByRole("menuitem", { name: "item 1" }); + // Jsdom does not report correct styles, see https://github.com/jsdom/jsdom/issues/2986. + // const item11 = within(menu).getByRole("menuitem", { name: "item 1-1" }); + // expect(item11).not.toBeVisible(); + expect(item1).toHaveAttribute("aria-current", "false"); + await user.click(item1); + expect(item1).toHaveAttribute("aria-current", "true"); +}); + +it("allows to drill out", async () => { + const { user } = plainRender( + {"item 1-1"}, + {"item 1-2"}, + ]} + upProps={{ label: "return" }} + > + item 1 + , + ]} + > + test + , + ); + const button = screen.getByRole("button", { name: "test" }); + await user.click(button); + const menu = screen.getByRole("menu", { name: "test menu" }); + const item1 = within(menu).getByRole("menuitem", { name: "item 1" }); + await user.click(item1); + expect(item1).toHaveAttribute("aria-current", "true"); + const back = within(menu).getByRole("menuitem", { name: "return" }); + await user.click(back); + expect(item1).not.toHaveAttribute("aria-current"); +}); + +it("calls the item action on click", async () => { + const action = jest.fn(); + const { user } = plainRender( + {"item 1"}, + + item 2 + , + ]} + > + test + , + ); + + const button = screen.getByRole("button", { name: "test" }); + await user.click(button); + const menu = screen.getByRole("menu"); + const item2 = within(menu).getByRole("menuitem", { name: "item 2" }); + await user.click(item2); + expect(action).toHaveBeenCalled(); +}); diff --git a/web/src/components/core/MenuButton.tsx b/web/src/components/core/MenuButton.tsx new file mode 100644 index 0000000000..0d8038bd2c --- /dev/null +++ b/web/src/components/core/MenuButton.tsx @@ -0,0 +1,210 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React, { useId, useMemo, useRef, useState } from "react"; +import { + MenuToggle, + DrilldownMenu, + MenuContent, + Divider, + MenuContainer, + Menu, + MenuList, + MenuItem, + MenuPopperProps, + MenuItemProps, + MenuToggleProps, +} from "@patternfly/react-core"; +import { _ } from "~/i18n"; + +/** + * PatternFly Menu crashes with ids generated by useId. + * + * Note that useId is actually generating an identifier that is a valid HTML id attribute but not a + * valid CSS identifier. Unfortunately, the PatternFly component is querying the node by using the + * querySelector JavaScript method without escaping the id (e.g., by using CSS.escape). + */ +function useMenuId() { + const id = useId(); + const menuId = useMemo(() => id.replaceAll(":", ""), [id]); + return menuId; +} + +interface MenuButtonItemProps extends Omit { + items?: React.ReactNode[]; + upProps?: { label?: string }; +} + +export function MenuButtonItem({ + items = [], + upProps = { label: _("Back") }, + onClick, + children, + ...props +}: React.PropsWithChildren): React.ReactNode { + const itemId = useId(); + const menuId = useMenuId(); + + const onDownClick = (event) => { + event.stopPropagation(); + onClick && onClick(event); + }; + + if (!items.length) { + return ( + + {children} + + ); + } + return ( + + e.stopPropagation()}> + {upProps.label} + + + {items} + + } + > + {children} + + ); +} + +export type MenuButtonProps = { + items?: React.ReactNode[]; + menuProps?: { + ["aria-label"]?: string; + closeOnClick?: boolean; + popperProps?: MenuPopperProps; + }; + toggleProps?: MenuToggleProps; +}; + +export default function MenuButton({ + items = [], + menuProps = {}, + toggleProps = {}, + children, +}: React.PropsWithChildren): React.ReactNode { + const menuRef = useRef(); + const toggleRef = useRef(); + const rootId = useMenuId(); + const [isOpen, setIsOpen] = useState(false); + const [menuDrilledIn, setMenuDrilledIn] = React.useState([]); + const [drilldownPath, setDrilldownPath] = React.useState([]); + const [activeMenu, setActiveMenu] = React.useState(rootId); + const [activeItemId, setActiveItemId] = React.useState(""); + const [menuHeights, setMenuHeights] = React.useState({}); + + menuProps = { popperProps: {}, closeOnClick: true, ...menuProps }; + + const resetState = () => { + setMenuDrilledIn([]); + setDrilldownPath([]); + setActiveMenu(rootId); + // Reset the height of the root menu. Otherwise, the memorized height could be wrong if the + // items of the root menu change. + if (!isOpen) setMenuHeights({ ...menuHeights, [rootId]: undefined }); + }; + + const toggle = () => { + setIsOpen(!isOpen); + resetState(); + }; + + const drillIn = ( + _: React.KeyboardEvent | React.MouseEvent, + fromMenuId: string, + toMenuId: string, + pathId: string, + ) => { + setMenuDrilledIn([...menuDrilledIn, fromMenuId]); + setDrilldownPath([...drilldownPath, pathId]); + setActiveMenu(toMenuId); + setActiveItemId(pathId); + }; + + const drillOut = (_: React.KeyboardEvent | React.MouseEvent, toMenuId: string) => { + const menuDrilledInSansLast = menuDrilledIn.slice(0, menuDrilledIn.length - 1); + const pathSansLast = drilldownPath.slice(0, drilldownPath.length - 1); + const lastPath = [...pathSansLast].pop(); + setMenuDrilledIn(menuDrilledInSansLast); + setDrilldownPath(pathSansLast); + setActiveMenu(toMenuId); + setActiveItemId(lastPath); + }; + + const setHeight = (menuId: string, height: number) => { + // FIXME: look for a better way to avoid test crashing because of this method + if (process.env.NODE_ENV === "test") return; + + if ( + menuHeights[menuId] === undefined || + (menuId !== rootId && menuHeights[menuId] !== height) + ) { + setMenuHeights({ ...menuHeights, [menuId]: height }); + } + }; + + return ( + + {children} + + } + menuRef={menuRef} + menu={ + menuProps.closeOnClick && setIsOpen(false)} + > + + {items} + + + } + /> + ); +} +MenuButton.Item = MenuButtonItem; diff --git a/web/src/components/overview/StorageSection.test.tsx b/web/src/components/overview/StorageSection.test.tsx index 91fc00026a..5eadb91715 100644 --- a/web/src/components/overview/StorageSection.test.tsx +++ b/web/src/components/overview/StorageSection.test.tsx @@ -93,7 +93,7 @@ describe("when the configuration contains one drive", () => { plainRender(); await screen.findByText(/Install using device/); - await screen.findByText(/\/dev\/sda, 500 GiB/); + await screen.findByText(/sda, 500 GiB/); await screen.findByText(/and deleting all its content/); }); diff --git a/web/src/components/storage/ConfigureDeviceMenu.test.tsx b/web/src/components/storage/ConfigureDeviceMenu.test.tsx index ec9d3e4a64..3879656931 100644 --- a/web/src/components/storage/ConfigureDeviceMenu.test.tsx +++ b/web/src/components/storage/ConfigureDeviceMenu.test.tsx @@ -109,7 +109,7 @@ describe("ConfigureDeviceMenu", () => { const { user } = plainRender(); const toggler = screen.getByRole("button", { name: /More devices/ }); await user.click(toggler); - const disksMenuItem = screen.getByRole("menuitem", { name: /disk to define/ }); + const disksMenuItem = screen.getByRole("menuitem", { name: "Add device menu" }); await user.click(disksMenuItem); const vdaItem = screen.getByRole("menuitem", { name: /vda/ }); await user.click(vdaItem); @@ -126,7 +126,7 @@ describe("ConfigureDeviceMenu", () => { const { user } = plainRender(); const toggler = screen.getByRole("button", { name: /More devices/ }); await user.click(toggler); - const disksMenuItem = screen.getByRole("menuitem", { name: /disk to define/ }); + const disksMenuItem = screen.getByRole("menuitem", { name: "Add device menu" }); await user.click(disksMenuItem); expect(screen.queryByRole("menuitem", { name: /vda/ })).toBeNull(); const vdbItem = screen.getByRole("menuitem", { name: /vdb/ }); @@ -145,7 +145,7 @@ describe("ConfigureDeviceMenu", () => { const { user } = plainRender(); const toggler = screen.getByRole("button", { name: /More devices/ }); await user.click(toggler); - const disksMenuItem = screen.getByRole("menuitem", { name: /disk to define/ }); + const disksMenuItem = screen.getByRole("menuitem", { name: "Add device menu" }); expect(disksMenuItem).toBeDisabled(); }); }); diff --git a/web/src/components/storage/ConfigureDeviceMenu.tsx b/web/src/components/storage/ConfigureDeviceMenu.tsx index 17b43e77d9..72763a9b5f 100644 --- a/web/src/components/storage/ConfigureDeviceMenu.tsx +++ b/web/src/components/storage/ConfigureDeviceMenu.tsx @@ -20,21 +20,10 @@ * find current contact information at www.suse.com. */ -import React, { useRef, useState } from "react"; +import React from "react"; import { useNavigate } from "react-router-dom"; -import { - MenuToggle, - Split, - Flex, - Label, - DrilldownMenu, - MenuContent, - Divider, - MenuContainer, - Menu, - MenuList, - MenuItem, -} from "@patternfly/react-core"; +import { Split, Flex, Label, Divider } from "@patternfly/react-core"; +import MenuButton, { MenuButtonItem } from "~/components/core/MenuButton"; import MenuDeviceDescription from "./MenuDeviceDescription"; import { useAvailableDevices } from "~/queries/storage"; import { useConfigModel, useModel } from "~/queries/storage/config-model"; @@ -60,7 +49,7 @@ const DisksDrillDownMenuItem = ({ drivesCount, devices, onDeviceClick, -}: DisksDrillDownMenuItemProps) => { +}: DisksDrillDownMenuItemProps): React.ReactNode => { const isDisabled = !devices.length; const disabledDescription = _("Already using all available disks"); @@ -79,41 +68,31 @@ const DisksDrillDownMenuItem = ({ : _("Select a disk to define partitions"); return ( - - - {_("Back")} - - - {devices.map((device) => ( - } - onClick={() => onDeviceClick(device.name)} - > - - {deviceLabel(device)} - - {device.systems.map((s, i) => ( - - ))} - - - - ))} - - } + items={devices.map((device) => ( + } + onClick={() => onDeviceClick(device.name)} + > + + {deviceLabel(device)} + + {device.systems.map((s, i) => ( + + ))} + + + + ))} > {title} - + ); }; @@ -127,118 +106,44 @@ const DisksDrillDownMenuItem = ({ * share the internal logic with other potential menus that could benefit from a similar * approach. */ -export default function ConfigureDeviceMenu() { +export default function ConfigureDeviceMenu(): React.ReactNode { const navigate = useNavigate(); const model = useConfigModel({ suspense: true }); const { addDrive } = useModel(); const allDevices = useAvailableDevices(); - const menuRef = useRef(); - const toggleRef = useRef(); - const [isOpen, setIsOpen] = useState(false); - const [menuDrilledIn, setMenuDrilledIn] = React.useState([]); - const [drilldownPath, setDrilldownPath] = React.useState([]); - const [activeMenu, setActiveMenu] = React.useState("root"); - const [menuHeights, setMenuHeights] = React.useState({}); - - const resetState = () => { - setMenuDrilledIn([]); - setDrilldownPath([]); - setActiveMenu("root"); - }; - - const toggle = () => { - setIsOpen(!isOpen); - resetState(); - }; - - const addDriveAndClose = (driveName) => { - addDrive(driveName); - setIsOpen(false); - resetState(); - }; const drivesNames = model.drives.map((d) => d.name); const drivesCount = drivesNames.length; const devices = allDevices.filter((d) => !drivesNames.includes(d.name)); - const drillIn = ( - _: React.KeyboardEvent | React.MouseEvent, - fromMenuId: string, - toMenuId: string, - pathId: string, - ) => { - setMenuDrilledIn([...menuDrilledIn, fromMenuId]); - setDrilldownPath([...drilldownPath, pathId]); - setActiveMenu(toMenuId); - }; - - const drillOut = (_: React.KeyboardEvent | React.MouseEvent, toMenuId: string) => { - const menuDrilledInSansLast = menuDrilledIn.slice(0, menuDrilledIn.length - 1); - const pathSansLast = drilldownPath.slice(0, drilldownPath.length - 1); - setMenuDrilledIn(menuDrilledInSansLast); - setDrilldownPath(pathSansLast); - setActiveMenu(toMenuId); - }; - - const setHeight = (menuId: string, height: number) => { - // FIXME: look for a better way to avoid test crashing because of this - // method - if (process.env.NODE_ENV === "test") return; - - if ( - menuHeights[menuId] === undefined || - (menuId !== "root" && menuHeights[menuId] !== height) - ) { - setMenuHeights({ ...menuHeights, [menuId]: height }); - } - }; - const lvmDescription = allDevices.length ? _("Define a new LVM on top of one or several disks") : _("Define a new LVM on the disk"); return ( - - {_("More devices")} - - } - menuRef={menuRef} - menu={ - , + , + navigate(PATHS.volumeGroup.add)} + description={lvmDescription} > - - - - - navigate(PATHS.volumeGroup.add)} - description={lvmDescription} - > - {_("Add LVM volume group")} - - - - - } - /> + {_("Add LVM volume group")} + , + ]} + > + {_("More devices")} + ); } diff --git a/web/src/components/storage/DeviceMenu.tsx b/web/src/components/storage/DeviceMenu.tsx index 6492b9ddbf..855461b742 100644 --- a/web/src/components/storage/DeviceMenu.tsx +++ b/web/src/components/storage/DeviceMenu.tsx @@ -21,7 +21,6 @@ */ import React, { useId, useRef, useState } from "react"; -import { Icon } from "~/components/layout"; import { Menu, MenuProps, @@ -34,13 +33,7 @@ import { const InlineMenuToggle = React.forwardRef( (props: MenuToggleProps, ref: React.Ref) => ( - } - innerRef={ref} - variant="plain" - className="agm-inline-menu-toggle" - {...props} - /> + ), ); diff --git a/web/src/components/storage/DriveDeviceMenu.tsx b/web/src/components/storage/DriveDeviceMenu.tsx new file mode 100644 index 0000000000..a99bbd0e6c --- /dev/null +++ b/web/src/components/storage/DriveDeviceMenu.tsx @@ -0,0 +1,394 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { Split, Flex, Label } from "@patternfly/react-core"; +import MenuButton, { MenuButtonItem } from "~/components/core/MenuButton"; +import MenuDeviceDescription from "./MenuDeviceDescription"; +import { useAvailableDevices } from "~/queries/storage"; +import { useDrive, useModel } from "~/queries/storage/config-model"; +import { useDrive as useDriveModel } from "~/hooks/storage/drive"; +import { useConvertToVolumeGroup } from "~/hooks/storage/volume-group"; +import * as driveUtils from "~/components/storage/utils/drive"; +import { deviceBaseName, deviceLabel, formattedPath } from "~/components/storage/utils"; +import { apiModel } from "~/api/storage/types"; +import { StorageDevice } from "~/types/storage"; +import { sprintf } from "sprintf-js"; +import { _, n_, formatList } from "~/i18n"; + +const driveBaseName = (device: StorageDevice): string => deviceBaseName(device, true); + +const UseOnlyOneOption = (drive: apiModel.Drive): boolean => { + const driveModel = useDrive(drive.name); + if (!driveModel) return false; + + const { isExplicitBoot, hasPv } = driveModel; + if (!driveUtils.hasFilesystem(drive) && (hasPv || isExplicitBoot)) return true; + + return driveUtils.hasReuse(drive); +}; + +type DiskSelectorTitleProps = { device: StorageDevice; isSelected: boolean }; + +const DiskSelectorTitle = ({ + device, + isSelected = false, +}: DiskSelectorTitleProps): React.ReactNode => { + const Name = () => (isSelected ? {deviceLabel(device)} : deviceLabel(device)); + const Systems = () => ( + + {device.systems.map((s, i) => ( + + ))} + + ); + + return ( + + + + + ); +}; + +const searchSelectorMultipleOptions = ( + devices: StorageDevice[], + selected: StorageDevice, + onChange: (name: StorageDevice["name"]) => void, +): React.ReactNode[] => { + return devices.map((device) => { + const isSelected = device.sid === selected.sid; + + return ( + } + onClick={() => onChange(device.name)} + > + + + ); + }); +}; + +const SearchSelectorSingleOption = ({ selected }: { selected: StorageDevice }): React.ReactNode => { + return ( + } + > + + + ); +}; + +const searchSelectorOptions = ( + drive: apiModel.Drive, + devices: StorageDevice[], + selected: StorageDevice, + onChange: (name: StorageDevice["name"]) => void, +): React.ReactNode[] => { + const onlyOneOption = UseOnlyOneOption(drive); + + if (onlyOneOption) return []; + + return searchSelectorMultipleOptions(devices, selected, onChange); +}; + +type DisksDrillDownMenuItemProps = { + drive: apiModel.Drive; + selected: StorageDevice; + onDeviceClick: (name: StorageDevice["name"]) => void; +}; + +/** + * Internal component holding the logic for rendering the disks drilldown menu + */ +const DisksDrillDownMenuItem = ({ + drive, + selected, + onDeviceClick, +}: DisksDrillDownMenuItemProps): React.ReactNode => { + /** @todo Replace the useDrive hook from /queries by the hook from /hooks. */ + const volumeGroups = useDriveModel(drive.name)?.getVolumeGroups() || []; + const onlyOneOption = UseOnlyOneOption(drive); + const devices = useAvailableDevices(); + const driveModel = useDrive(drive.name); + if (!driveModel) return; + + const { isBoot, isExplicitBoot, hasPv } = driveModel; + const vgName = volumeGroups[0]?.vgName; + + const mainText = (): string => { + if (onlyOneOption) { + return _("Selected disk (cannot be changed)"); + } + + if (!driveUtils.hasFilesystem(drive)) { + return _("Select a disk to configure"); + } + + if (driveUtils.hasRoot(drive)) { + return _("Select a disk to install the system"); + } + + const mountPaths = drive.partitions + .filter((p) => !p.name) + .map((p) => formattedPath(p.mountPath)); + + return sprintf( + // TRANSLATORS: %s is a list of formatted mount points like '"/", "/var" and "swap"' (or a + // single mount point in the singular case). + _("Select a disk to create %s"), + formatList(mountPaths), + ); + }; + + const extraText = (): string => { + const name = driveBaseName(selected); + + if (driveUtils.hasReuse(drive)) { + // The current device will be the only option to choose from + return _("This uses existing partitions at the disk"); + } + + if (!driveUtils.hasFilesystem(drive)) { + // The current device will be the only option to choose from + if (hasPv) { + if (volumeGroups.length > 1) { + if (isExplicitBoot) { + return _( + "This disk will contain the configured LVM groups and any partition needed to boot", + ); + } + return _("This disk will contain the configured LVM groups"); + } + if (isExplicitBoot) { + return sprintf( + // TRANSLATORS: %s is the name of the LVM + _("This disk will contain the LVM group '%s' and any partition needed to boot"), + vgName, + ); + } + + // TRANSLATORS: %s is the name of the LVM + return sprintf(_("This disk will contain the LVM group '%s'"), vgName); + } + + // The current device will be the only option to choose from + if (isExplicitBoot) { + return _("This disk will contain any partition needed for booting"); + } + } + + if (hasPv) { + if (volumeGroups.length > 1) { + if (isExplicitBoot) { + return sprintf( + // TRANSLATORS: %s is the name of the disk (eg. sda) + _("%s will still contain the configured LVM groups and any partition needed to boot"), + name, + ); + } + + // TRANSLATORS: %s is the name of the disk (eg. sda) + return sprintf(_("The configured LVM groups will remain at %s"), name); + } + + if (isExplicitBoot) { + return sprintf( + // TRANSLATORS: %1$s is the name of the disk (eg. sda) and %2$s the name of the LVM + _("%1$s will still contain the LVM group '%2$s' and any partition needed to boot"), + name, + vgName, + ); + } + + return sprintf( + // TRANSLATORS: %1$s is the name of the LVM and %2$s the name of the disk (eg. sda) + _("The LVM group '%1$s' will remain at %2$s"), + vgName, + name, + ); + } + + if (isExplicitBoot) { + // TRANSLATORS: %s is the name of the disk (eg. sda) + return sprintf(_("Partitions needed for booting will remain at %s"), name); + } + + if (isBoot) { + return _("Partitions needed for booting will also be adapted"); + } + }; + + const text = mainText(); + + return ( + + {text} + + ); +}; + +type RemoveDriveOptionProps = { drive: apiModel.Drive }; + +const RemoveDriveOption = ({ drive }: RemoveDriveOptionProps): React.ReactNode => { + const driveModel = useDrive(drive.name); + const { hasAdditionalDrives } = useModel(); + + if (!driveModel) return; + + const { isExplicitBoot, hasPv, delete: deleteDrive } = driveModel; + + // When no additional drives has been added, the "Do not use" button can be confusing so it is + // omitted for all drives. + if (!hasAdditionalDrives) return; + + let description; + const isDisabled = isExplicitBoot || hasPv; + + if (isExplicitBoot) { + if (hasPv) { + description = _("The disk is used for LVM and boot"); + } else { + description = _("The disk is used for booting"); + } + } else { + if (hasPv) { + description = _("The disk is used for LVM"); + } else { + description = _("Remove the configuration for this disk"); + } + } + + return ( + + {_("Do not use")} + + ); +}; + +type NewVgOptionProps = DriveDeviceMenuProps; + +const NewVgOption = ({ drive, selected }: NewVgOptionProps): React.ReactNode => { + const convertToVg = useConvertToVolumeGroup(); + /** @todo Replace the useDrive hook from /queries by the hook from /hooks. */ + const vgs = useDriveModel(drive.name)?.getVolumeGroups() || []; + const mountPaths = drive.partitions.filter((p) => !p.name).map((p) => formattedPath(p.mountPath)); + + const titleText = () => { + if (vgs.length) { + // TRANSLATORS: %s is the short name of a disk, like 'sda' + return sprintf(_("Create another LVM volume group on %s"), driveBaseName(selected)); + } + + // TRANSLATORS: %s is the short name of a disk, like 'sda' + return sprintf(_("Create LVM volume group on %s"), driveBaseName(selected)); + }; + + const descriptionText = () => { + if (mountPaths.length) { + return sprintf( + n_( + // TRANSLATORS: %s is a list of formatted mount points like '"/", "/var" and "swap"' (or a + // single mount point in the singular case). + "%s will be created as a logical volume", + "%s will be created as logical volumes", + mountPaths.length, + ), + formatList(mountPaths), + ); + } + }; + + return ( + convertToVg(drive.name)} + itemId="lvm" + description={descriptionText()} + > + + {titleText()} + + + ); +}; + +export type DriveDeviceMenuProps = { drive: apiModel.Drive; selected: StorageDevice }; + +/** + * Menu that provides options for users to configure the device used by a given drive. + * + * It uses a drilled-down menu approach for disks, making the available options less + * overwhelming by presenting them in a more organized manner. + */ +export default function DriveDeviceMenu({ + drive, + selected, +}: DriveDeviceMenuProps): React.ReactNode { + const driveHandler = useDrive(drive.name); + const changeDriveTarget = (newDriveName: string) => { + driveHandler.switch(newDriveName); + }; + + return ( + , + , + , + ]} + > + {{deviceLabel(selected, true)}} + + ); +} diff --git a/web/src/components/storage/DriveEditor.test.tsx b/web/src/components/storage/DriveEditor.test.tsx index 320d6c19a4..08186b50b7 100644 --- a/web/src/components/storage/DriveEditor.test.tsx +++ b/web/src/components/storage/DriveEditor.test.tsx @@ -235,9 +235,9 @@ describe("RemoveDriveOption", () => { it("allows users to delete regular drives", async () => { const { user } = plainRender(); - const driveButton = screen.getByRole("button", { name: "Drive" }); + const driveButton = screen.getByRole("button", { name: "sdb, 1 KiB" }); await user.click(driveButton); - const drivesMenu = screen.getByRole("menu"); + const drivesMenu = screen.getByRole("menu", { name: "Device /dev/sdb menu" }); const deleteDriveButton = within(drivesMenu).getByRole("menuitem", { name: /Do not use/, }); @@ -248,13 +248,13 @@ describe("RemoveDriveOption", () => { it("does not allow users to delete drives explicitly used to boot", async () => { const { user } = plainRender(); - const driveButton = screen.getByRole("button", { name: "Drive" }); + const driveButton = screen.getByRole("button", { name: "sda, 1 KiB" }); await user.click(driveButton); - const drivesMenu = screen.getByRole("menu"); + const drivesMenu = screen.getByRole("menu", { name: "Device /dev/sda menu" }); const deleteDriveButton = within(drivesMenu).queryByRole("menuitem", { name: /Do not use/, }); - expect(deleteDriveButton).not.toBeInTheDocument(); + expect(deleteDriveButton).toBeDisabled(); }); }); @@ -266,9 +266,9 @@ describe("RemoveDriveOption", () => { it("does not allow users to delete regular drives", async () => { const { user } = plainRender(); - const driveButton = screen.getByRole("button", { name: "Drive" }); + const driveButton = screen.getByRole("button", { name: "sdb, 1 KiB" }); await user.click(driveButton); - const drivesMenu = screen.getByRole("menu"); + const drivesMenu = screen.getByRole("menu", { name: "Device /dev/sdb menu" }); const deleteDriveButton = within(drivesMenu).queryByRole("menuitem", { name: /Do not use/, }); @@ -278,9 +278,9 @@ describe("RemoveDriveOption", () => { it("does not allow users to delete drives explicitly used to boot", async () => { const { user } = plainRender(); - const driveButton = screen.getByRole("button", { name: "Drive" }); + const driveButton = screen.getByRole("button", { name: "sda, 1 KiB" }); await user.click(driveButton); - const drivesMenu = screen.getByRole("menu"); + const drivesMenu = screen.getByRole("menu", { name: "Device /dev/sda menu" }); const deleteDriveButton = within(drivesMenu).queryByRole("menuitem", { name: /Do not use/, }); diff --git a/web/src/components/storage/DriveEditor.tsx b/web/src/components/storage/DriveEditor.tsx index 20d526fbe1..db519db7ca 100644 --- a/web/src/components/storage/DriveEditor.tsx +++ b/web/src/components/storage/DriveEditor.tsx @@ -22,22 +22,19 @@ import React from "react"; import { useNavigate, generatePath } from "react-router-dom"; -import { _, formatList } from "~/i18n"; -import { sprintf } from "sprintf-js"; -import { baseName, deviceLabel, formattedPath, SPACE_POLICIES } from "~/components/storage/utils"; -import { useAvailableDevices } from "~/queries/storage"; +import { _ } from "~/i18n"; +import { baseName, SPACE_POLICIES } from "~/components/storage/utils"; import { apiModel } from "~/api/storage/types"; import { StorageDevice } from "~/types/storage"; import { STORAGE as PATHS } from "~/routes/paths"; -import { useDrive, useModel } from "~/queries/storage/config-model"; -import { useDrive as useDriveModel } from "~/hooks/storage/drive"; +import { useDrive } from "~/queries/storage/config-model"; import * as driveUtils from "~/components/storage/utils/drive"; import { contentDescription } from "~/components/storage/utils/device"; +import DriveDeviceMenu from "~/components/storage/DriveDeviceMenu"; import DeviceMenu from "~/components/storage/DeviceMenu"; import DeviceHeader from "~/components/storage/DeviceHeader"; import MountPathMenuItem from "~/components/storage/MountPathMenuItem"; import { MenuHeader } from "~/components/core"; -import MenuDeviceDescription from "./MenuDeviceDescription"; import { Card, CardBody, @@ -126,276 +123,6 @@ const SpacePolicySelector = ({ drive, driveDevice }: DriveEditorProps) => { ); }; -const SearchSelectorIntro = ({ drive }: { drive: apiModel.Drive }) => { - /** @todo Replace the useDrive hook from /queries by the hook from /hooks. */ - const volumeGroups = useDriveModel(drive.name)?.getVolumeGroups() || []; - const driveModel = useDrive(drive.name); - if (!driveModel) return; - - const { isBoot, isExplicitBoot, hasPv } = driveModel; - const vgName = volumeGroups[0]?.vgName; - - const mainText = (): string => { - if (driveUtils.hasReuse(drive)) { - // The current device will be the only option to choose from - return _("This uses existing partitions at the device"); - } - - if (!driveUtils.hasFilesystem(drive)) { - // The current device will be the only option to choose from - if (hasPv) { - if (volumeGroups.length > 1) { - if (isExplicitBoot) { - return _( - "This device will contain the configured LVM groups and any partition needed to boot", - ); - } - return _("This device will contain the configured LVM groups"); - } - if (isExplicitBoot) { - return sprintf( - // TRANSLATORS: %s is the name of the LVM - _("This device will contain the LVM group '%s' and any partition needed to boot"), - vgName, - ); - } - - // TRANSLATORS: %s is the name of the LVM - return sprintf(_("This device will contain the LVM group '%s'"), vgName); - } - - // The current device will be the only option to choose from - if (isExplicitBoot) { - return _("This device will contain any partition needed for booting"); - } - - // I guess 'create new LVM' is not a reasonable option here - return _("Select a device to configure"); - } - - if (driveUtils.hasRoot(drive)) { - return _("Select a device to install the system"); - } - - const mountPaths = drive.partitions - .filter((p) => !p.name) - .map((p) => formattedPath(p.mountPath)); - - return sprintf( - // TRANSLATORS: %s is a list of formatted mount points like '"/", "/var" and "swap"' (or a - // single mount point in the singular case). - _("Select a device to create %s"), - formatList(mountPaths), - ); - }; - - const extraText = (): string => { - // Nothing to add in these cases - if (driveUtils.hasReuse(drive)) return; - if (!driveUtils.hasFilesystem(drive)) return; - - const name = baseName(drive.name); - - if (hasPv) { - if (volumeGroups.length > 1) { - if (isExplicitBoot) { - return sprintf( - // TRANSLATORS: %s is the name of the disk (eg. sda) - _("%s will still contain the configured LVM groups and any partition needed to boot"), - name, - ); - } - - // TRANSLATORS: %s is the name of the disk (eg. sda) - return sprintf(_("The configured LVM groups will remain at %s"), name); - } - - if (isExplicitBoot) { - return sprintf( - // TRANSLATORS: %1$s is the name of the disk (eg. sda) and %2$s the name of the LVM - _("%1$s will still contain the LVM group '%2$s' and any partition needed to boot"), - name, - vgName, - ); - } - - return sprintf( - // TRANSLATORS: %1$s is the name of the LVM and %2$s the name of the disk (eg. sda) - _("The LVM group '%1$s' will remain at %2$s"), - vgName, - name, - ); - } - - if (isExplicitBoot) { - // TRANSLATORS: %s is the name of the disk (eg. sda) - return sprintf(_("Partitions needed for booting will remain at %s"), name); - } - - if (isBoot) { - return _("Partitions needed for booting will also be adapted"); - } - }; - - return ; -}; - -const DiskSelectorTitle = ({ device, isSelected = false }) => { - const Name = () => (isSelected ? {deviceLabel(device)} : deviceLabel(device)); - const Systems = () => ( - - {device.systems.map((s, i) => ( - - ))} - - ); - - return ( - - - - - ); -}; - -const SearchSelectorMultipleOptions = ({ selected, withNewVg = false, onChange }) => { - const navigate = useNavigate(); - const devices = useAvailableDevices(); - - const NewVgOption = () => { - if (withNewVg) - return ( - navigate(PATHS.root)} - itemId="lvm" - description={_("The configured partitions will be created as logical volumes")} - > - - {_("New LVM volume group")} - - - ); - }; - - return ( - <> - {devices.map((device) => { - const isSelected = device.sid === selected.sid; - - return ( - } - onClick={() => onChange(device.name)} - > - - - ); - })} - - - ); -}; - -const SearchSelectorSingleOption = ({ selected }) => { - return ( - } - > - - - ); -}; - -const SearchSelectorOptions = ({ drive, selected, onChange }) => { - const driveModel = useDrive(drive.name); - if (!driveModel) return; - - const { isExplicitBoot, hasPv } = driveModel; - // const boot = isExplicitBoot(drive.name); - - if (driveUtils.hasReuse(drive)) return ; - - if (!driveUtils.hasFilesystem(drive)) { - if (hasPv || isExplicitBoot) { - return ; - } - - return ; - } - - // TODO: use withNewVg prop once LVM is added. - return ; -}; - -const SearchSelector = ({ drive, selected, onChange }) => { - return ( - }> - - - - ); -}; - -const RemoveDriveOption = ({ drive }) => { - const driveModel = useDrive(drive.name); - const { hasAdditionalDrives } = useModel(); - - if (!driveModel) return; - - const { isExplicitBoot, hasPv, delete: deleteDrive } = driveModel; - - // When no additional drives has been added, the "Do not use" button can be confusing so it is - // omitted for all drives. - if (!hasAdditionalDrives) return; - - // FIXME: in these two cases the button should likely be present, but disabled and with an - // explanation of why those particular drive definitions cannot be removed. - if (isExplicitBoot) return; - if (hasPv) return; - - return ( - <> - - - {_("Do not use")} - - - ); -}; - -const DriveSelector = ({ drive, selected, toggleAriaLabel }) => { - const driveHandler = useDrive(drive.name); - const onDriveChange = (newDriveName: string) => { - driveHandler.switch(newDriveName); - }; - - return ( - {deviceLabel(selected)}} - ariaLabel={toggleAriaLabel} - activeItemId={selected.sid} - > - - - - - - ); -}; - const DriveHeader = ({ drive, driveDevice }: DriveEditorProps) => { const { isBoot, hasPv } = useDrive(drive.name); @@ -452,12 +179,10 @@ const DriveHeader = ({ drive, driveDevice }: DriveEditorProps) => { // TRANSLATORS: %s will be replaced by the device name and its size - "/dev/sda, 20 GiB" return _("Use %s"); }; - // TRANSLATORS: a disk drive - const toggleAriaLabel = _("Drive"); return ( - + ); }; diff --git a/web/src/components/storage/LvmPage.test.tsx b/web/src/components/storage/LvmPage.test.tsx index 90b2728adb..044663d3c0 100644 --- a/web/src/components/storage/LvmPage.test.tsx +++ b/web/src/components/storage/LvmPage.test.tsx @@ -158,8 +158,8 @@ describe("LvmPage", () => { const { user } = installerRender(); const name = screen.getByRole("textbox", { name: "Name" }); const disks = screen.getByRole("group", { name: "Disks" }); - const sdaCheckbox = within(disks).getByRole("checkbox", { name: "/dev/sda, 1 KiB" }); - const sdbCheckbox = within(disks).getByRole("checkbox", { name: "/dev/sdb, 1 KiB" }); + const sdaCheckbox = within(disks).getByRole("checkbox", { name: "sda, 1 KiB" }); + const sdbCheckbox = within(disks).getByRole("checkbox", { name: "sdb, 1 KiB" }); const moveMountPointsCheckbox = screen.getByRole("checkbox", { name: /Move the mount points currently configured at the selected disks to logical volumes/, }); @@ -186,7 +186,7 @@ describe("LvmPage", () => { it("allows configuring a new LVM volume group (moving mount points)", async () => { const { user } = installerRender(); const disks = screen.getByRole("group", { name: "Disks" }); - const sdbCheckbox = within(disks).getByRole("checkbox", { name: "/dev/sdb, 1 KiB" }); + const sdbCheckbox = within(disks).getByRole("checkbox", { name: "sdb, 1 KiB" }); const moveMountPointsCheckbox = screen.getByRole("checkbox", { name: /Move the mount points currently configured at the selected disks to logical volumes/, }); @@ -205,7 +205,7 @@ describe("LvmPage", () => { const { user } = installerRender(); const name = screen.getByRole("textbox", { name: "Name" }); const disks = screen.getByRole("group", { name: "Disks" }); - const sdaCheckbox = within(disks).getByRole("checkbox", { name: "/dev/sda, 1 KiB" }); + const sdaCheckbox = within(disks).getByRole("checkbox", { name: "sda, 1 KiB" }); const acceptButton = screen.getByRole("button", { name: "Accept" }); // Unselect sda @@ -279,7 +279,7 @@ describe("LvmPage", () => { const { user } = installerRender(); const name = screen.getByRole("textbox", { name: "Name" }); const disks = screen.getByRole("group", { name: "Disks" }); - const sdaCheckbox = within(disks).getByRole("checkbox", { name: "/dev/sda, 1 KiB" }); + const sdaCheckbox = within(disks).getByRole("checkbox", { name: "sda, 1 KiB" }); const acceptButton = screen.getByRole("button", { name: "Accept" }); // Let's clean the default given name @@ -301,7 +301,7 @@ describe("LvmPage", () => { it("pre-fills form with the current volume group configuration", async () => { installerRender(); const name = screen.getByRole("textbox", { name: "Name" }); - const sdaCheckbox = screen.getByRole("checkbox", { name: "/dev/sda, 1 KiB" }); + const sdaCheckbox = screen.getByRole("checkbox", { name: "sda, 1 KiB" }); expect(name).toHaveValue("fakeRootVg"); expect(sdaCheckbox).toBeChecked(); }); diff --git a/web/src/components/storage/LvmPage.tsx b/web/src/components/storage/LvmPage.tsx index 2fb746e362..cc50599102 100644 --- a/web/src/components/storage/LvmPage.tsx +++ b/web/src/components/storage/LvmPage.tsx @@ -167,7 +167,7 @@ export default function LvmPage() { {deviceLabel(device)}} + label={{deviceLabel(device, true)}} description={ diff --git a/web/src/components/storage/PartitionPage.tsx b/web/src/components/storage/PartitionPage.tsx index fe569848d2..9763d697b2 100644 --- a/web/src/components/storage/PartitionPage.tsx +++ b/web/src/components/storage/PartitionPage.tsx @@ -494,10 +494,10 @@ function TargetOptionLabel({ value }: TargetOptionLabelProps): React.ReactNode { const device = useDevice(); if (value === NEW_PARTITION) { - // TRANSLATORS: %1$s is a disk name (eg. "/dev/sda") and %2$s is its size (eg. "250 GiB") - return sprintf(_("As a new partition on %1$s (%2$s)"), device.name, deviceSize(device.size)); + // TRANSLATORS: %s is a disk name with its size (eg. "sda, 10 GiB" + return sprintf(_("As a new partition on %s"), deviceLabel(device, true)); } else { - return sprintf(_("Using partition %s"), value); + return sprintf(_("Using partition %s"), baseName(value, true)); } } diff --git a/web/src/components/storage/VolumeGroupEditor.tsx b/web/src/components/storage/VolumeGroupEditor.tsx index f13487f829..a043351050 100644 --- a/web/src/components/storage/VolumeGroupEditor.tsx +++ b/web/src/components/storage/VolumeGroupEditor.tsx @@ -22,10 +22,12 @@ import React from "react"; import { useNavigate, generatePath } from "react-router-dom"; -import { _ } from "~/i18n"; +import { sprintf } from "sprintf-js"; +import { _, n_, formatList } from "~/i18n"; import { STORAGE as PATHS } from "~/routes/paths"; import { apiModel } from "~/api/storage/types"; import { model } from "~/types/storage"; +import { baseName, formattedPath } from "~/components/storage/utils"; import { contentDescription } from "~/components/storage/utils/volume-group"; import { useVolumeGroup, useDeleteVolumeGroup } from "~/hooks/storage/volume-group"; import { useDeleteLogicalVolume } from "~/hooks/storage/logical-volume"; @@ -47,15 +49,42 @@ import spacingStyles from "@patternfly/react-styles/css/utilities/Spacing/spacin const DeleteVgOption = ({ vg }: { vg: model.VolumeGroup }) => { const deleteVolumeGroup = useDeleteVolumeGroup(); + const lvs = vg.logicalVolumes.map((lv) => formattedPath(lv.mountPath)); + const targetDevices = vg.getTargetDevices(); + const convert = targetDevices.length === 1 && !!lvs.length; + let description; + + if (lvs.length) { + if (convert) { + const diskName = baseName(targetDevices[0].name, true); + description = sprintf( + n_( + // TRANSLATORS: %1$s is a list of formatted mount points like '"/", "/var" and "swap"' (or a + // single mount point in the singular case). %2$s is a disk name like sda. + "%1$s will be created as a partition at %2$s", + "%1$s will be created as partitions at %2$s", + lvs.length, + ), + formatList(lvs), + diskName, + ); + } else { + description = n_( + "The logical volume will also be deleted", + "The logical volumes will also be deleted", + lvs.length, + ); + } + } return ( deleteVolumeGroup(vg.vgName)} + onClick={() => deleteVolumeGroup(vg.vgName, convert)} > {_("Delete volume group")} diff --git a/web/src/components/storage/utils.test.ts b/web/src/components/storage/utils.test.ts index 814571a1e1..5a9ddc2467 100644 --- a/web/src/components/storage/utils.test.ts +++ b/web/src/components/storage/utils.test.ts @@ -172,15 +172,15 @@ describe("deviceBaseName", () => { }); describe("deviceLabel", () => { - it("returns the device name and size", () => { + it("returns the device basename and size", () => { const result = deviceLabel(sda); - expect(result).toEqual("/dev/sda, 1 KiB"); + expect(result).toEqual("sda, 1 KiB"); }); - it("returns only the device name if the device has no size", () => { + it("returns only the device basename if the device has no size", () => { const device = { ...sda, size: 0 }; const result = deviceLabel(device); - expect(result).toEqual("/dev/sda"); + expect(result).toEqual("sda"); }); }); diff --git a/web/src/components/storage/utils.ts b/web/src/components/storage/utils.ts index 39b31bc7a0..0e72a8b0eb 100644 --- a/web/src/components/storage/utils.ts +++ b/web/src/components/storage/utils.ts @@ -177,25 +177,43 @@ const parseToBytes = (size: string | number): number => { return Math.trunc(value); }; +const TRUNCATE_MAX_LENGTH = 17; + /** * Base name for a full path + * + * FIXME: The truncate param allows to generate a shorter representation that fits into + * the interface, but that's a temporary solution. The right way to make the strings fit + * into the responsive interface would be Patternfly's Truncate component. */ -const baseName = (name: string): string => { - return name.split("/").pop(); +const baseName = (name: string, truncate?: boolean): string => { + const base = name.split("/").pop(); + + if (!truncate || base.length <= TRUNCATE_MAX_LENGTH) return base; + + // Simplistic approach as a first implementation. Anyways, we plan to replace this with + // the usage of Patternfly's Truncate in the mid-term. + const limit1 = Math.ceil((TRUNCATE_MAX_LENGTH - 1) / 2.0); + const limit2 = base.length - Math.floor((TRUNCATE_MAX_LENGTH - 1) / 2.0); + return base.slice(0, limit1) + "…" + base.slice(limit2); }; /** * Base name of a device. + * + * FIXME: See note at baseName about the usage of truncate. */ -const deviceBaseName = (device: StorageDevice): string => { - return baseName(device.name); +const deviceBaseName = (device: StorageDevice, truncate?: boolean): string => { + return baseName(device.name, truncate); }; /** * Generates the label for the given device + * + * FIXME: See note at baseName about the usage of truncate. */ -const deviceLabel = (device: StorageDevice): string => { - const name = device.name; +const deviceLabel = (device: StorageDevice, truncate?: boolean): string => { + const name = deviceBaseName(device, truncate); const size = device.size; return size ? `${name}, ${deviceSize(size)}` : name; diff --git a/web/src/helpers/storage/api-model.ts b/web/src/helpers/storage/api-model.ts index 491d550772..514fb43e6f 100644 --- a/web/src/helpers/storage/api-model.ts +++ b/web/src/helpers/storage/api-model.ts @@ -72,10 +72,19 @@ function buildLogicalVolumeFromPartition(partition: apiModel.Partition): apiMode }; } +function buildPartitionFromLogicalVolume(lv: apiModel.LogicalVolume): apiModel.Partition { + return { + mountPath: lv.mountPath, + filesystem: lv.filesystem, + size: lv.size, + }; +} + export { copyApiModel, buildVolumeGroup, buildLogicalVolume, buildLogicalVolumeName, buildLogicalVolumeFromPartition, + buildPartitionFromLogicalVolume, }; diff --git a/web/src/helpers/storage/volume-group.ts b/web/src/helpers/storage/volume-group.ts index 1841f2aaf4..94fe51fb3a 100644 --- a/web/src/helpers/storage/volume-group.ts +++ b/web/src/helpers/storage/volume-group.ts @@ -26,6 +26,7 @@ import { copyApiModel, buildVolumeGroup, buildLogicalVolumeFromPartition, + buildPartitionFromLogicalVolume, } from "~/helpers/storage/api-model"; import { data } from "~/types/storage"; @@ -63,6 +64,29 @@ function addVolumeGroup( return apiModel; } +function newVgName(apiModel: apiModel.Config): string { + const vgs = (apiModel.volumeGroups || []).filter((vg) => vg.vgName.match(/^system\d*$/)); + + if (!vgs.length) return "system"; + + const numbers = vgs.map((vg) => parseInt(vg.vgName.substring(6)) || 0); + return `system${Math.max(...numbers) + 1}`; +} + +function driveToVolumeGroup(apiModel: apiModel.Config, driveName: string): apiModel.Config { + apiModel = copyApiModel(apiModel); + + const drive = (apiModel.drives || []).find((d) => d.name === driveName); + if (!drive) return apiModel; + + const volumeGroup = buildVolumeGroup({ vgName: newVgName(apiModel), targetDevices: [driveName] }); + movePartitions(drive, volumeGroup); + apiModel.volumeGroups ||= []; + apiModel.volumeGroups.push(volumeGroup); + + return apiModel; +} + function editVolumeGroup( apiModel: apiModel.Config, vgName: string, @@ -84,6 +108,26 @@ function editVolumeGroup( return apiModel; } +function volumeGroupToDrive(apiModel: apiModel.Config, vgName: string): apiModel.Config { + apiModel = copyApiModel(apiModel); + + const index = (apiModel.volumeGroups || []).findIndex((v) => v.vgName === vgName); + if (index === -1) return apiModel; + + const targetDevice = apiModel.volumeGroups[index].targetDevices[0]; + if (!targetDevice) return apiModel; + + const drive = (apiModel.drives || []).find((d) => d.name === targetDevice); + if (!drive) return apiModel; + + const logicalVolumes = apiModel.volumeGroups[index].logicalVolumes || []; + apiModel.volumeGroups.splice(index, 1); + const partitions = drive.partitions || []; + drive.partitions = [...partitions, ...logicalVolumes.map(buildPartitionFromLogicalVolume)]; + + return apiModel; +} + function deleteVolumeGroup(apiModel: apiModel.Config, vgName: string): apiModel.Config { apiModel = copyApiModel(apiModel); @@ -93,11 +137,21 @@ function deleteVolumeGroup(apiModel: apiModel.Config, vgName: string): apiModel. const targetDevices = apiModel.volumeGroups[index].targetDevices || []; apiModel.volumeGroups.splice(index, 1); + if (!targetDevices.length) return apiModel; + + let deletedApiModel = copyApiModel(apiModel); targetDevices.forEach((d) => { - apiModel = deleteIfUnused(apiModel, d); + deletedApiModel = deleteIfUnused(deletedApiModel, d); }); - return apiModel; + // Do not delete the underlying drives if that results in an empty configuration + return deletedApiModel.drives.length ? deletedApiModel : apiModel; } -export { addVolumeGroup, editVolumeGroup, deleteVolumeGroup }; +export { + addVolumeGroup, + editVolumeGroup, + deleteVolumeGroup, + volumeGroupToDrive, + driveToVolumeGroup, +}; diff --git a/web/src/hooks/storage/volume-group.ts b/web/src/hooks/storage/volume-group.ts index 892e269041..04325628f0 100644 --- a/web/src/hooks/storage/volume-group.ts +++ b/web/src/hooks/storage/volume-group.ts @@ -21,7 +21,13 @@ */ import { useApiModel, useUpdateApiModel } from "~/hooks/storage/api-model"; -import { addVolumeGroup, editVolumeGroup, deleteVolumeGroup } from "~/helpers/storage/volume-group"; +import { + addVolumeGroup, + editVolumeGroup, + deleteVolumeGroup, + volumeGroupToDrive, + driveToVolumeGroup, +} from "~/helpers/storage/volume-group"; import { QueryHookOptions } from "~/types/queries"; import { model, data } from "~/types/storage"; import { useModel } from "~/hooks/storage/model"; @@ -52,13 +58,33 @@ function useEditVolumeGroup(options?: QueryHookOptions): EditVolumeGroupFn { }; } -type DeleteVolumeGroupFn = (vgName: string) => void; +type DeleteVolumeGroupFn = (vgName: string, moveToDrive: boolean) => void; function useDeleteVolumeGroup(options?: QueryHookOptions): DeleteVolumeGroupFn { const apiModel = useApiModel(options); const updateApiModel = useUpdateApiModel(); - return (vgName: string) => updateApiModel(deleteVolumeGroup(apiModel, vgName)); + return (vgName: string, moveToDrive: boolean) => { + updateApiModel( + moveToDrive ? volumeGroupToDrive(apiModel, vgName) : deleteVolumeGroup(apiModel, vgName), + ); + }; +} + +type ConvertToVolumeGroupFn = (driveName: string) => void; + +function useConvertToVolumeGroup(options?: QueryHookOptions): ConvertToVolumeGroupFn { + const apiModel = useApiModel(options); + const updateApiModel = useUpdateApiModel(); + return (driveName: string) => { + updateApiModel(driveToVolumeGroup(apiModel, driveName)); + }; } -export { useVolumeGroup, useAddVolumeGroup, useEditVolumeGroup, useDeleteVolumeGroup }; -export type { AddVolumeGroupFn, EditVolumeGroupFn, DeleteVolumeGroupFn }; +export { + useVolumeGroup, + useAddVolumeGroup, + useEditVolumeGroup, + useDeleteVolumeGroup, + useConvertToVolumeGroup, +}; +export type { AddVolumeGroupFn, EditVolumeGroupFn, DeleteVolumeGroupFn, ConvertToVolumeGroupFn };