From cceea91fa0f4fde7eefcf540b00e171f6657e962 Mon Sep 17 00:00:00 2001 From: Bartosz Leper Date: Mon, 7 Apr 2025 19:36:25 +0200 Subject: [PATCH] Remember current standard editor tab (#53644) * Remember current standard editor tab * review --- .../src/Roles/RoleEditor/RoleEditor.test.tsx | 5 +-- .../StandardEditor/StandardEditor.tsx | 34 +++++++------------ .../StandardEditor/standardmodel.ts | 9 +++++ .../StandardEditor/useStandardModel.ts | 26 +++++++++++++- 4 files changed, 48 insertions(+), 26 deletions(-) diff --git a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.test.tsx b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.test.tsx index 89c58ed8c9c..46cd3d90056 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.test.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/RoleEditor.test.tsx @@ -112,13 +112,10 @@ test('rendering and switching tabs for new role', async () => { expect(screen.getByRole('button', { name: 'Create Role' })).toBeEnabled(); await user.click(getStandardEditorTab()); - await screen.findByLabelText('Role Name *'); + await screen.findByLabelText('Max Session TTL'); expect( screen.queryByRole('button', { name: /Reset to Standard Settings/i }) ).not.toBeInTheDocument(); - await forwardToTab('Resources'); - await forwardToTab('Admin Rules'); - await forwardToTab('Options'); expect(screen.getByRole('button', { name: 'Create Role' })).toBeEnabled(); }, 10000); diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StandardEditor.tsx b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StandardEditor.tsx index d2340851f22..354f27a8a0f 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StandardEditor.tsx +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/StandardEditor.tsx @@ -16,8 +16,7 @@ * along with this program. If not, see . */ -import { produce } from 'immer'; -import { useCallback, useId, useState } from 'react'; +import { useCallback, useId } from 'react'; import styled from 'styled-components'; import { Box, ButtonPrimary, ButtonSecondary, Flex } from 'design'; @@ -37,6 +36,7 @@ import { OptionsModel, roleEditorModelToRole, StandardEditorModel, + StandardEditorTab, } from './standardmodel'; import { ActionType, StandardModelDispatcher } from './useStandardModel'; @@ -60,20 +60,10 @@ export const StandardEditor = ({ dispatch, }: StandardEditorProps) => { const isEditing = !!originalRole; - const { roleModel, validationResult } = standardEditorModel; + const { roleModel, validationResult, currentTab, disabledTabs } = + standardEditorModel; const modelValid = validationResult.isValid; - enum StandardEditorTab { - Overview, - Resources, - AdminRules, - Options, - } - - const [currentTab, setCurrentTab] = useState(StandardEditorTab.Overview); - const [disabledTabs, setDisabledTabs] = useState( - isEditing ? [false, false, false, false] : [false, true, true, true] - ); const idPrefix = useId(); const validator = useValidation(); @@ -104,6 +94,13 @@ export const StandardEditor = ({ [dispatch] ); + const setCurrentTab = useCallback( + (newTab: StandardEditorTab) => { + dispatch({ type: ActionType.SetCurrentTab, payload: newTab }); + }, + [dispatch] + ); + const validateAndGoToNextTab = useCallback(() => { const nextTabIndex = currentTab + 1; const valid = validate(); @@ -112,12 +109,7 @@ export const StandardEditor = ({ } validator.reset(); setCurrentTab(nextTabIndex); - setDisabledTabs(prevEnabledTabs => - produce(prevEnabledTabs, et => { - et[nextTabIndex] = false; - }) - ); - }, [currentTab, validate, validator, setCurrentTab, setDisabledTabs]); + }, [currentTab, validate, validator, setCurrentTab]); const goToPreviousTab = useCallback( () => setCurrentTab(currentTab - 1), @@ -136,7 +128,7 @@ export const StandardEditor = ({ return { key: tab, title: tabTitles[tab], - disabled: disabledTabs[tab], + disabled: disabledTabs.has(tab), controls: tabElementIDs[tab], status: error ? validationErrorTabStatus : undefined, }; diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.ts b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.ts index 33c65554145..61b922997ee 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.ts +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/standardmodel.ts @@ -53,6 +53,13 @@ import { import { RoleEditorModelValidationResult } from './validation'; import { defaultOptions } from './withDefaults'; +export enum StandardEditorTab { + Overview, + Resources, + AdminRules, + Options, +} + export type StandardEditorModel = { /** * The role model. Can be undefined if there was an unhandled error when @@ -70,6 +77,8 @@ export type StandardEditorModel = { * role. */ validationResult?: RoleEditorModelValidationResult; + currentTab: StandardEditorTab; + disabledTabs: Set; }; /** diff --git a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/useStandardModel.ts b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/useStandardModel.ts index 9927e6e1012..f22deed9309 100644 --- a/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/useStandardModel.ts +++ b/web/packages/teleport/src/Roles/RoleEditor/StandardEditor/useStandardModel.ts @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -import { current, original } from 'immer'; +import { current, enableMapSet, original } from 'immer'; import { Dispatch } from 'react'; import { useImmerReducer } from 'use-immer'; @@ -38,11 +38,15 @@ import { RoleEditorModel, roleToRoleEditorModel, StandardEditorModel, + StandardEditorTab, } from './standardmodel'; import { validateRoleEditorModel } from './validation'; const logger = new Logger('useStandardModel'); +// Enable support for the Set type in Immer. We use it for `disabledTabs`. +enableMapSet(); + /** * Creates a standard model state and returns an array composed of the state * and an action dispatcher that can be used to change it. Since the conversion @@ -56,6 +60,7 @@ export const useStandardModel = ( useImmerReducer(reduce, originalRole, initializeState); const initializeState = (originalRole?: Role): StandardEditorModel => { + const isEditing = !!originalRole; const role = originalRole ?? newRole(); const roleModel = safelyConvertRoleToEditorModel(role); return { @@ -64,6 +69,14 @@ const initializeState = (originalRole?: Role): StandardEditorModel => { isDirty: !originalRole, // New role is dirty by default. validationResult: roleModel && validateRoleEditorModel(roleModel, undefined, undefined), + currentTab: StandardEditorTab.Overview, + disabledTabs: isEditing + ? new Set() + : new Set([ + StandardEditorTab.Resources, + StandardEditorTab.AdminRules, + StandardEditorTab.Options, + ]), }; }; @@ -82,6 +95,7 @@ const safelyConvertRoleToEditorModel = ( export type StandardModelDispatcher = Dispatch; export enum ActionType { + SetCurrentTab = 'SetCurrentTab', SetModel = 'SetModel', ResetToStandard = 'ResetToStandard', SetMetadata = 'SetMetadata', @@ -99,6 +113,7 @@ export enum ActionType { } type StandardModelAction = + | SetCurrentTabAction | SetModelAction | ResetToStandardAction | SetMetadataAction @@ -115,6 +130,10 @@ type StandardModelAction = | EnableValidationAction; /** Sets the entire model. */ +type SetCurrentTabAction = { + type: ActionType.SetCurrentTab; + payload: StandardEditorTab; +}; type SetModelAction = { type: ActionType.SetModel; payload: RoleEditorModel }; type ResetToStandardAction = { type: ActionType.ResetToStandard; @@ -175,6 +194,11 @@ const reduce = ( // This reduce uses Immer, so we modify the model draft directly. // TODO(bl-nero): add immutability to the model data types. switch (type) { + case ActionType.SetCurrentTab: + state.currentTab = payload; + state.disabledTabs.delete(payload); + break; + case ActionType.SetModel: state.roleModel = payload; break;