From db4b6dc556f193cf610ab1eb34351da44fbbb339 Mon Sep 17 00:00:00 2001 From: Jake Howell Date: Thu, 6 Aug 2026 11:14:30 +1000 Subject: [PATCH] refactor(site): de-MUI custom role create/edit form (#27820) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate the organization custom role create/edit form off MUI onto shared site components. - Replace `TextField` with `FormField` for name and display name - Replace permission checkboxes with the shared shadcn `Checkbox` (including indeterminate) - Replace the “show advanced permissions” control with a `Switch` + `Label` - Update the toggle story to query by accessible checkbox role/name --- .../CreateEditRolePageView.stories.tsx | 45 ++- .../CreateEditRolePageView.tsx | 265 +++++++++--------- 2 files changed, 174 insertions(+), 136 deletions(-) diff --git a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx index 90a44e9cfc..dca8956d87 100644 --- a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx +++ b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.stories.tsx @@ -105,12 +105,51 @@ export const ToggleParentCheckbox: Story = { play: async ({ canvasElement }) => { const user = userEvent.setup(); const canvas = within(canvasElement); - const checkbox = await canvas - .getByTestId("audit_log") - .getElementsByTagName("input")[0]; + const checkbox = canvas.getByRole("checkbox", { name: "audit_log" }); await user.click(checkbox); await expect(checkbox).toBeChecked(); await user.click(checkbox); await expect(checkbox).not.toBeChecked(); }, }; + +export const ToggleAdvancedPermissions: Story = { + args: { + ...Default.args, + role: undefined, + }, + play: async ({ canvasElement }) => { + const user = userEvent.setup(); + const canvas = within(canvasElement); + + // Advanced-only resources are hidden until the switch is enabled. + expect( + canvas.queryByRole("checkbox", { name: "api_key" }), + ).not.toBeInTheDocument(); + + const [toggle] = canvas.getAllByRole("switch", { + name: "Show advanced permissions", + }); + await user.click(toggle); + + // Enabling the switch reveals advanced resources and flips the label. + await expect( + canvas.getByRole("checkbox", { name: "api_key" }), + ).toBeInTheDocument(); + const [enabledToggle] = canvas.getAllByRole("switch", { + name: "Hide advanced permissions", + }); + await expect(enabledToggle).toBeChecked(); + + await user.click(enabledToggle); + + // Disabling the switch hides advanced resources again. + await expect( + canvas.queryByRole("checkbox", { name: "api_key" }), + ).not.toBeInTheDocument(); + const [disabledToggle] = canvas.getAllByRole("switch", { + name: "Show advanced permissions", + }); + await expect(disabledToggle).not.toBeChecked(); + }, +}; diff --git a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx index fdb437fc8d..0f9437e1b7 100644 --- a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx +++ b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx @@ -1,9 +1,5 @@ -import Checkbox from "@mui/material/Checkbox"; -import FormControlLabel from "@mui/material/FormControlLabel"; -import TextField from "@mui/material/TextField"; import { useFormik } from "formik"; -import { EyeIcon, EyeOffIcon } from "lucide-react"; -import { type ChangeEvent, type FC, useState } from "react"; +import { type FC, useId, useState } from "react"; import { useNavigate } from "react-router"; import * as Yup from "yup"; import { isApiValidationError } from "#/api/errors"; @@ -18,18 +14,21 @@ import type { } from "#/api/typesGenerated"; import { ErrorAlert } from "#/components/Alert/ErrorAlert"; import { Button } from "#/components/Button/Button"; +import { Checkbox } from "#/components/Checkbox/Checkbox"; import { FormFields, FormFooter, VerticalForm } from "#/components/Form/Form"; +import { FormField } from "#/components/FormField/FormField"; +import { Label } from "#/components/Label/Label"; import { SettingsHeader, SettingsHeaderDescription, SettingsHeaderTitle, } from "#/components/SettingsHeader/SettingsHeader"; import { Spinner } from "#/components/Spinner/Spinner"; +import { Switch } from "#/components/Switch/Switch"; import { Table, TableBody, TableCell, - TableFooter, TableHead, TableHeader, TableRow, @@ -114,22 +113,22 @@ const CreateEditRolePageView: FC = ({ )} - - > & { values: Role }; @@ -197,11 +202,7 @@ const ActionCheckboxes: FC = ({ ? RBACResourceActions : filteredRBACResourceActions; - const handleActionCheckChange = async ( - e: ChangeEvent, - form: ReturnType> & { values: Role }, - ) => { - const { name, checked } = e.currentTarget; + const handleActionCheckChange = async (name: string, checked: boolean) => { const [resource_type, action] = name.split(":"); const newPermissions = checked @@ -222,13 +223,10 @@ const ActionCheckboxes: FC = ({ }; const handleResourceCheckChange = async ( - e: ChangeEvent, - form: ReturnType> & { values: Role }, + resource: RBACResource, + checked: boolean, indeterminate: boolean, ) => { - const { name, checked } = e.currentTarget; - const resource = name as RBACResource; - const resourceActionsForResource = resourceActions[resource] || {}; const newCheckedActions = @@ -253,61 +251,55 @@ const ActionCheckboxes: FC = ({ }; return ( - - - - Permission - - - - - - - {Object.entries(resourceActions).map(([resourceKey, value]) => { - return ( - a.resource_type === resourceKey, - )} - resourceKey={resourceKey} - value={value} - form={form} - handleActionCheckChange={handleActionCheckChange} - handleResourceCheckChange={handleResourceCheckChange} - /> - ); - })} - - - - - - - - -
+ <> + + + + Permission + + + + + + + {Object.entries(resourceActions).map(([resourceKey, value]) => { + if (!isRBACResource(resourceKey)) { + return null; + } + return ( + a.resource_type === resourceKey, + )} + resourceKey={resourceKey} + value={value} + handleActionCheckChange={handleActionCheckChange} + handleResourceCheckChange={handleResourceCheckChange} + /> + ); + })} + +
+ + ); }; interface PermissionCheckboxGroupProps { checkedActions: readonly Permission[]; - resourceKey: string; + resourceKey: RBACResource; value: Partial>; - form: ReturnType> & { values: Role }; - handleActionCheckChange: ( - e: ChangeEvent, - form: ReturnType> & { values: Role }, - ) => Promise; + handleActionCheckChange: (name: string, checked: boolean) => Promise; handleResourceCheckChange: ( - e: ChangeEvent, - form: ReturnType> & { values: Role }, + resource: RBACResource, + checked: boolean, indeterminate: boolean, ) => Promise; } @@ -316,50 +308,62 @@ const PermissionCheckboxGroup: FC = ({ checkedActions, resourceKey, value, - form, handleActionCheckChange, handleResourceCheckChange, }) => { + const actionCount = Object.keys(value).length; + const isResourceChecked = checkedActions.length === actionCount; + const isResourceIndeterminate = + checkedActions.length > 0 && checkedActions.length < actionCount; + return ( - +
  • - 0 && - checkedActions.length < Object.keys(value).length - } - data-testid={`${resourceKey}`} - onChange={(e) => - handleResourceCheckChange( - e, - form, - checkedActions.length > 0 && - checkedActions.length < Object.keys(value).length, - ) - } - /> - {resourceKey} -
      - {Object.entries(value).map(([actionKey, value]) => ( -
    • - - - ResourceActionComparator(p, resourceKey, actionKey), - )} - onChange={(e) => handleActionCheckChange(e, form)} - /> - {actionKey} - - {value} -
    • - ))} +
      + + handleResourceCheckChange( + resourceKey, + checked === true, + isResourceIndeterminate, + ) + } + /> + {resourceKey} +
      +
        + {Object.entries(value).map(([actionKey, description]) => { + const actionName = `${resourceKey}:${actionKey}`; + const isActionChecked = checkedActions.some((p) => + ResourceActionComparator(p, resourceKey, actionKey), + ); + + return ( +
      • + + + handleActionCheckChange(actionName, checked === true) + } + /> + {actionKey} + + + {description} + +
      • + ); + })}
      @@ -367,37 +371,32 @@ const PermissionCheckboxGroup: FC = ({ ); }; -interface ShowAllResourcesCheckboxProps { +interface ShowAllResourcesSwitchProps { showAllResources: boolean; setShowAllResources: React.Dispatch>; } -const ShowAllResourcesCheckbox: FC = ({ +const ShowAllResourcesSwitch: FC = ({ showAllResources, setShowAllResources, }) => { + const id = useId(); + return ( - setShowAllResources(e.currentTarget.checked)} - checkedIcon={} - icon={} - /> - } - label={ - - {showAllResources - ? "Hide advanced permissions" - : "Show advanced permissions"} - - } - /> +
      + + +
      ); };