From ff377cd70461912e9e390c178cb718afc69254db Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Tue, 28 May 2024 14:01:59 +0200 Subject: [PATCH] chore: new project dialog code cleanup 1 (#7113) This PR implements some initial cleanup work for the new project creation dialog. The primary focus here is to remove unused props and to use the same logic for the configuration buttons regardless of the content (mode, stickiness, envs, change requests). --- .../CreateProject/ChangeRequestTable.tsx | 1 - .../Project/CreateProject/CreateProject.tsx | 13 - .../CreateProjectDialog.tsx | 6 - .../Project/CreateProject/NewProjectForm.tsx | 38 +- .../CreateProject/SelectionButton.styles.tsx | 6 + .../Project/CreateProject/SelectionButton.tsx | 427 +++++++++--------- 6 files changed, 231 insertions(+), 260 deletions(-) diff --git a/frontend/src/component/project/Project/CreateProject/ChangeRequestTable.tsx b/frontend/src/component/project/Project/CreateProject/ChangeRequestTable.tsx index 7e193055d6..b92c791c74 100644 --- a/frontend/src/component/project/Project/CreateProject/ChangeRequestTable.tsx +++ b/frontend/src/component/project/Project/CreateProject/ChangeRequestTable.tsx @@ -14,7 +14,6 @@ import { ConditionallyRender } from 'component/common/ConditionallyRender/Condit import GeneralSelect from 'component/common/GeneralSelect/GeneralSelect'; import KeyboardArrowDownOutlined from '@mui/icons-material/KeyboardArrowDownOutlined'; import { useTheme } from '@mui/material/styles'; -// import { PROJECT_CHANGE_REQUEST_WRITE } from '../../../../providers/AccessProvider/permissions'; const StyledBox = styled(Box)(({ theme }) => ({ padding: theme.spacing(1), diff --git a/frontend/src/component/project/Project/CreateProject/CreateProject.tsx b/frontend/src/component/project/Project/CreateProject/CreateProject.tsx index 95b292f6f4..520b9cf23e 100644 --- a/frontend/src/component/project/Project/CreateProject/CreateProject.tsx +++ b/frontend/src/component/project/Project/CreateProject/CreateProject.tsx @@ -15,7 +15,6 @@ import { GO_BACK } from 'constants/navigate'; import { usePlausibleTracker } from 'hooks/usePlausibleTracker'; import { Button, styled } from '@mui/material'; import { useUiFlag } from 'hooks/useUiFlag'; -import { useState } from 'react'; const CREATE_PROJECT_BTN = 'CREATE_PROJECT_BTN'; @@ -35,14 +34,10 @@ const CreateProject = () => { projectName, projectDesc, projectMode, - projectEnvironments, - projectChangeRequestConfiguration, setProjectMode, setProjectId, setProjectName, setProjectDesc, - setProjectEnvironments, - updateProjectChangeRequestConfig, getCreateProjectPayload, clearErrors, validateProjectId, @@ -56,14 +51,6 @@ const CreateProject = () => { return ; } - const generalDocumentation = - 'Projects allows you to group feature flags together in the management UI.'; - - const [documentation, setDocumentation] = useState(generalDocumentation); - - const clearDocumentationOverride = () => - setDocumentation(generalDocumentation); - const { createProject, loading } = useProjectApi(); const handleSubmit = async (e: Event) => { diff --git a/frontend/src/component/project/Project/CreateProject/CreateProjectDialog/CreateProjectDialog.tsx b/frontend/src/component/project/Project/CreateProject/CreateProjectDialog/CreateProjectDialog.tsx index 11568efa43..808d00a05c 100644 --- a/frontend/src/component/project/Project/CreateProject/CreateProjectDialog/CreateProjectDialog.tsx +++ b/frontend/src/component/project/Project/CreateProject/CreateProjectDialog/CreateProjectDialog.tsx @@ -60,14 +60,12 @@ export const CreateProjectDialog = ({ projectEnvironments, projectChangeRequestConfiguration, setProjectMode, - setProjectId, setProjectName, setProjectDesc, setProjectEnvironments, updateProjectChangeRequestConfig, getCreateProjectPayload, clearErrors, - validateProjectId, validateName, setProjectStickiness, projectStickiness, @@ -151,7 +149,6 @@ export const CreateProjectDialog = ({ projectId={projectId} projectEnvironments={projectEnvironments} setProjectEnvironments={setProjectEnvironments} - setProjectId={setProjectId} projectName={projectName} projectStickiness={projectStickiness} projectChangeRequestConfiguration={ @@ -166,9 +163,6 @@ export const CreateProjectDialog = ({ setProjectName={setProjectName} projectDesc={projectDesc} setProjectDesc={setProjectDesc} - mode='Create' - clearErrors={clearErrors} - validateProjectId={validateProjectId} overrideDocumentation={setDocumentation} clearDocumentationOverride={clearDocumentationOverride} > diff --git a/frontend/src/component/project/Project/CreateProject/NewProjectForm.tsx b/frontend/src/component/project/Project/CreateProject/NewProjectForm.tsx index c10664969a..b5ed31dbb1 100644 --- a/frontend/src/component/project/Project/CreateProject/NewProjectForm.tsx +++ b/frontend/src/component/project/Project/CreateProject/NewProjectForm.tsx @@ -3,7 +3,7 @@ import Input from 'component/common/Input/Input'; import type { ProjectMode } from '../hooks/useProjectEnterpriseSettingsForm'; import { ReactComponent as ProjectIcon } from 'assets/icons/projectIconSmall.svg'; import { - MultiselectList, + MultiSelectList, SingleSelectList, TableSelect, } from './SelectionButton'; @@ -80,8 +80,6 @@ type FormProps = { projectName: string; projectDesc: string; projectStickiness: string; - featureLimit?: string; - featureCount?: number; projectMode: string; projectEnvironments: Set; projectChangeRequestConfiguration: Record< @@ -90,10 +88,8 @@ type FormProps = { >; setProjectStickiness: React.Dispatch>; setProjectEnvironments: (envs: Set) => void; - setProjectId: React.Dispatch>; setProjectName: React.Dispatch>; setProjectDesc: React.Dispatch>; - setFeatureLimit?: React.Dispatch>; setProjectMode: React.Dispatch>; updateProjectChangeRequestConfig: { disableChangeRequests: (env: string) => void; @@ -101,9 +97,6 @@ type FormProps = { }; handleSubmit: (e: any) => void; errors: { [key: string]: string }; - mode: 'Create' | 'Edit'; - clearErrors: () => void; - validateProjectId: () => void; overrideDocumentation: (args: { text: string; icon: ReactNode }) => void; clearDocumentationOverride: () => void; }; @@ -119,20 +112,14 @@ export const NewProjectForm: React.FC = ({ projectStickiness, projectEnvironments, projectChangeRequestConfiguration, - featureLimit, - featureCount, projectMode, setProjectMode, setProjectEnvironments, - setProjectId, setProjectName, setProjectDesc, setProjectStickiness, updateProjectChangeRequestConfig, - setFeatureLimit, errors, - mode, - clearErrors, overrideDocumentation, clearDocumentationOverride, }) => { @@ -183,6 +170,15 @@ export const NewProjectForm: React.FC = ({ : numberOfConfiguredChangeRequestEnvironments === 1 ? `1 environment configured` : 'Configure change requests'; + + const availableChangeRequestEnvironments = ( + projectEnvironments.size === 0 + ? activeEnvironments + : activeEnvironments.filter((env) => + projectEnvironments.has(env.name), + ) + ).map(({ name, type }) => ({ name, type })); + return ( { @@ -235,7 +231,7 @@ export const NewProjectForm: React.FC = ({ - ({ @@ -316,15 +312,9 @@ export const NewProjectForm: React.FC = ({ description={ selectionButtonData.changeRequests.text } - disabled={projectEnvironments.size === 0} - activeEnvironments={activeEnvironments - .filter((env) => - projectEnvironments.has(env.name), - ) - .map((env) => ({ - name: env.name, - type: env.type, - }))} + activeEnvironments={ + availableChangeRequestEnvironments + } updateProjectChangeRequestConfiguration={ updateProjectChangeRequestConfig } diff --git a/frontend/src/component/project/Project/CreateProject/SelectionButton.styles.tsx b/frontend/src/component/project/Project/CreateProject/SelectionButton.styles.tsx index 53c0517645..f58d97e116 100644 --- a/frontend/src/component/project/Project/CreateProject/SelectionButton.styles.tsx +++ b/frontend/src/component/project/Project/CreateProject/SelectionButton.styles.tsx @@ -74,3 +74,9 @@ export const ScrollContainer = styled('div')(({ theme }) => ({ width: '100%', overflow: 'auto', })); + +export const ButtonLabel = styled('span', { + shouldForwardProp: (prop) => prop !== 'labelWidth', +})<{ labelWidth?: string }>(({ labelWidth }) => ({ + width: labelWidth || 'unset', +})); diff --git a/frontend/src/component/project/Project/CreateProject/SelectionButton.tsx b/frontend/src/component/project/Project/CreateProject/SelectionButton.tsx index e8b1754c4f..2539542e38 100644 --- a/frontend/src/component/project/Project/CreateProject/SelectionButton.tsx +++ b/frontend/src/component/project/Project/CreateProject/SelectionButton.tsx @@ -1,14 +1,14 @@ import Search from '@mui/icons-material/Search'; import { v4 as uuidv4 } from 'uuid'; import { - Box, - Button, - InputAdornment, - List, - ListItemText, - styled, -} from '@mui/material'; -import { type FC, type ReactNode, useRef, useState, useMemo } from 'react'; + type FC, + type ReactNode, + useRef, + useState, + useMemo, + type PropsWithChildren, +} from 'react'; +import { Box, Button, InputAdornment, List, ListItemText } from '@mui/material'; import { StyledCheckbox, StyledDropdown, @@ -17,7 +17,7 @@ import { StyledDropdownSearch, TableSearchInput, HiddenDescription, - ScrollContainer, + ButtonLabel, } from './SelectionButton.styles'; import { ChangeRequestTable } from './ChangeRequestTable'; @@ -94,24 +94,30 @@ type CombinedSelectProps = { description: string; // visually hidden, for assistive tech }; -const CombinedSelect: FC = ({ - options, - onChange, +const CombinedSelect: FC< + PropsWithChildren<{ + button: { label: string; icon: ReactNode; labelWidth?: string }; + onOpen?: () => void; + onClose?: () => void; + description: string; + preventOpen?: boolean; + anchorEl: HTMLDivElement | null | undefined; + setAnchorEl: (el: HTMLDivElement | null | undefined) => void; + }> +> = ({ button, - search, - multiselect, onOpen = () => {}, onClose = () => {}, description, + children, + preventOpen, + anchorEl, + setAnchorEl, }) => { const ref = useRef(null); - const [anchorEl, setAnchorEl] = useState(); - const [searchText, setSearchText] = useState(''); const descriptionId = uuidv4(); - const [recentlyClosed, setRecentlyClosed] = useState(false); const open = () => { - setSearchText(''); setAnchorEl(ref.current); onOpen(); }; @@ -121,30 +127,6 @@ const CombinedSelect: FC = ({ onClose(); }; - const onSelection = (selected: string) => { - onChange(selected); - if (!multiselect) { - handleClose(); - setRecentlyClosed(true); - // this is a hack to prevent the button from being - // auto-clicked after you select an item by pressing enter - // in the search bar for single-select lists. - setTimeout(() => setRecentlyClosed(false), 1); - } - }; - - const { listRefs, handleSelection } = useSelectionManagement({ - handleToggle: (selected: string) => () => onSelection(selected), - }); - - const filteredOptions = options?.filter((option) => - option.label.toLowerCase().includes(searchText.toLowerCase()), - ); - - const ButtonLabel = styled('span')(() => ({ - width: button.labelWidth || 'unset', - })); - return ( <> @@ -153,12 +135,14 @@ const CombinedSelect: FC = ({ color='primary' startIcon={button.icon} onClick={() => { - if (!recentlyClosed) { + if (!preventOpen) { open(); } }} > - {button.label} + + {button.label} + = ({ {description} - setSearchText(event.target.value)} - label={search.label} - hideLabel - placeholder={search.placeholder} - autoFocus - InputProps={{ - startAdornment: ( - - - - ), - }} - inputRef={(el) => { - listRefs.current[0] = el; - }} - onKeyDown={(event) => - handleSelection(event, 0, filteredOptions) - } - /> - - {filteredOptions.map((option, index) => { - const labelId = `checkbox-list-label-${option.value}`; - - return ( - { - onSelection(option.value); - }} - ref={(el) => { - listRefs.current[index + 1] = el; - }} - onKeyDown={(event) => - handleSelection( - event, - index + 1, - filteredOptions, - ) - } - > - {multiselect ? ( - - ) : null} - - - ); - })} - + {children} ); }; -type MultiselectListProps = Pick< - CombinedSelectProps, - 'options' | 'button' | 'search' | 'onOpen' | 'onClose' | 'description' -> & { - selectedOptions: Set; - onChange: (values: Set) => void; -}; - -export const MultiselectList: FC = ({ - selectedOptions, +const DropdownList: FC = ({ + options, onChange, - ...rest + search, + multiselect, }) => { - // todo: add "select all" and "deselect all" + const [searchText, setSearchText] = useState(''); - const handleToggle = (value: string) => { - if (selectedOptions.has(value)) { - selectedOptions.delete(value); - } else { - selectedOptions.add(value); - } - - onChange(new Set(selectedOptions)); + const onSelection = (selected: string) => { + onChange(selected); }; + const { listRefs, handleSelection } = useSelectionManagement({ + handleToggle: (selected: string) => () => onSelection(selected), + }); + + const filteredOptions = options?.filter((option) => + option.label.toLowerCase().includes(searchText.toLowerCase()), + ); + return ( - + <> + setSearchText(event.target.value)} + label={search.label} + hideLabel + placeholder={search.placeholder} + autoFocus + InputProps={{ + startAdornment: ( + + + + ), + }} + inputRef={(el) => { + listRefs.current[0] = el; + }} + onKeyDown={(event) => + handleSelection(event, 0, filteredOptions) + } + /> + + {filteredOptions.map((option, index) => { + const labelId = `checkbox-list-label-${option.value}`; + + return ( + { + onSelection(option.value); + }} + ref={(el) => { + listRefs.current[index + 1] = el; + }} + onKeyDown={(event) => + handleSelection( + event, + index + 1, + filteredOptions, + ) + } + > + {multiselect ? ( + + ) : null} + + + ); + })} + + ); }; @@ -301,8 +273,73 @@ type SingleSelectListProps = Pick< | 'description' >; -export const SingleSelectList: FC = (props) => { - return ; +export const SingleSelectList: FC = ({ + onChange, + ...props +}) => { + const [anchorEl, setAnchorEl] = useState(); + const [recentlyClosed, setRecentlyClosed] = useState(false); + + const handleChange = (value: any) => { + onChange(value); + setAnchorEl(null); + props.onClose && props.onClose(); + + setRecentlyClosed(true); + // this is a hack to prevent the button from being + // auto-clicked after you select an item by pressing enter + // in the search bar for single-select lists. + setTimeout(() => setRecentlyClosed(false), 1); + }; + + return ( + + + + ); +}; + +type MultiselectListProps = Pick< + CombinedSelectProps, + 'options' | 'button' | 'search' | 'onOpen' | 'onClose' | 'description' +> & { + selectedOptions: Set; + onChange: (values: Set) => void; +}; + +export const MultiSelectList: FC = ({ + selectedOptions, + onChange, + ...rest +}) => { + const [anchorEl, setAnchorEl] = useState(); + + const handleToggle = (value: string) => { + if (selectedOptions.has(value)) { + selectedOptions.delete(value); + } else { + selectedOptions.add(value); + } + + onChange(new Set(selectedOptions)); + }; + + return ( + + + + ); }; type TableSelectProps = Pick< @@ -321,17 +358,17 @@ type TableSelectProps = Pick< string, { requiredApprovals: number } >; - disabled: boolean; }; + export const TableSelect: FC = ({ button, - disabled, search, projectChangeRequestConfiguration, updateProjectChangeRequestConfiguration, activeEnvironments, onOpen = () => {}, onClose = () => {}, + ...props }) => { const configured = useMemo(() => { return Object.fromEntries( @@ -365,21 +402,9 @@ export const TableSelect: FC = ({ updateProjectChangeRequestConfiguration.disableChangeRequests(name); }; - const ref = useRef(null); const [anchorEl, setAnchorEl] = useState(); const [searchText, setSearchText] = useState(''); - const open = () => { - setSearchText(''); - setAnchorEl(ref.current); - onOpen(); - }; - - const handleClose = () => { - setAnchorEl(null); - onClose(); - }; - const filteredEnvs = tableEnvs.filter((env) => env.name.toLowerCase().includes(searchText.toLowerCase()), ); @@ -399,66 +424,36 @@ export const TableSelect: FC = ({ } }; - const ButtonLabel = styled('span')(() => ({ - width: button.labelWidth || 'unset', - })); - return ( - <> - - - - + setSearchText(event.target.value)} + hideLabel + label={search.label} + placeholder={search.placeholder} + autoFocus + InputProps={{ + startAdornment: ( + + + + ), }} - transformOrigin={{ - vertical: 'top', - horizontal: 'left', - }} - > - - setSearchText(event.target.value)} - hideLabel - label={search.label} - placeholder={search.placeholder} - autoFocus - InputProps={{ - startAdornment: ( - - - - ), - }} - onKeyDown={toggleTopItem} - /> - - - - - - + onKeyDown={toggleTopItem} + /> + + ); };