From b84699f563261e410152194b9e53a91d73c892eb Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Mon, 24 Mar 2025 08:39:35 +0100 Subject: [PATCH] refactor(1-3439): extract shared components and styling from Env Accordion Body to common (#9590) Extracts the shared strategy list and list item into the `common` folder instead of living in the environment accordion body file. Also takes the disabled strategy handling that we use for `StrategySeparator` and moves it into the file itself. It might be something we want to decorate manually in the future, but we don't for now, so this was the most straight-forward way to make it work. --- .../common/StrategyList/StrategyList.tsx | 18 +++++ .../common/StrategyList/StrategyListItem.tsx | 15 ++++ .../StrategySeparator/StrategySeparator.tsx | 15 ++-- .../EnvironmentAccordionBody.tsx | 68 +++++-------------- .../ReleasePlanMilestone.tsx | 15 ++-- .../PlaygroundResultStrategyLists.tsx | 21 +++--- .../MilestoneCard/MilestoneCard.tsx | 15 ++-- 7 files changed, 83 insertions(+), 84 deletions(-) create mode 100644 frontend/src/component/common/StrategyList/StrategyList.tsx create mode 100644 frontend/src/component/common/StrategyList/StrategyListItem.tsx diff --git a/frontend/src/component/common/StrategyList/StrategyList.tsx b/frontend/src/component/common/StrategyList/StrategyList.tsx new file mode 100644 index 0000000000..196bd31ca0 --- /dev/null +++ b/frontend/src/component/common/StrategyList/StrategyList.tsx @@ -0,0 +1,18 @@ +import { styled } from '@mui/material'; + +export const StrategyList = styled('ol')(({ theme }) => ({ + listStyle: 'none', + padding: 0, + margin: 0, + + '& > li': { + paddingBlock: theme.spacing(2.5), + position: 'relative', + }, + '& > li + li': { + borderTop: `1px solid ${theme.palette.divider}`, + }, + '& > li:first-of-type': { + paddingTop: theme.spacing(1), + }, +})); diff --git a/frontend/src/component/common/StrategyList/StrategyListItem.tsx b/frontend/src/component/common/StrategyList/StrategyListItem.tsx new file mode 100644 index 0000000000..ffab804601 --- /dev/null +++ b/frontend/src/component/common/StrategyList/StrategyListItem.tsx @@ -0,0 +1,15 @@ +import { type Theme, styled } from '@mui/material'; + +export const releasePlanBackground = (theme: Theme) => + theme.palette.background.elevation2; +export const strategyBackground = (theme: Theme) => + theme.palette.background.elevation1; + +export const StrategyListItem = styled('li')<{ + 'data-type'?: 'release-plan' | 'strategy'; +}>(({ theme, ...props }) => ({ + background: + props['data-type'] === 'release-plan' + ? releasePlanBackground(theme) + : strategyBackground(theme), +})); diff --git a/frontend/src/component/common/StrategySeparator/StrategySeparator.tsx b/frontend/src/component/common/StrategySeparator/StrategySeparator.tsx index 682c9443e2..88e97a0f84 100644 --- a/frontend/src/component/common/StrategySeparator/StrategySeparator.tsx +++ b/frontend/src/component/common/StrategySeparator/StrategySeparator.tsx @@ -1,4 +1,5 @@ import { styled } from '@mui/material'; +import { disabledStrategyClassName } from '../StrategyItemContainer/disabled-strategy-utils'; const Chip = styled('div')(({ theme }) => ({ padding: theme.spacing(0.75, 1), @@ -11,12 +12,14 @@ const Chip = styled('div')(({ theme }) => ({ borderRadius: theme.shape.borderRadiusLarge, backgroundColor: theme.palette.secondary.border, left: theme.spacing(4), + + // if the strategy it's applying to is disabled + [`&:has(+ * .${disabledStrategyClassName}, + .${disabledStrategyClassName})`]: + { + filter: 'grayscale(.8)', + }, })); -export const StrategySeparator = ({ className }: { className?: string }) => { - return ( - - OR - - ); +export const StrategySeparator = () => { + return OR; }; diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/EnvironmentAccordionBody.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/EnvironmentAccordionBody.tsx index 1468fdbeb6..32e3130dea 100644 --- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/EnvironmentAccordionBody.tsx +++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/EnvironmentAccordionBody.tsx @@ -4,7 +4,7 @@ import { useEffect, useState, } from 'react'; -import { Alert, Pagination, type Theme, styled } from '@mui/material'; +import { Alert, Pagination, styled } from '@mui/material'; import useFeatureStrategyApi from 'hooks/api/actions/useFeatureStrategyApi/useFeatureStrategyApi'; import { formatUnknownError } from 'utils/formatUnknownError'; import useToast from 'hooks/useToast'; @@ -22,7 +22,12 @@ import { useReleasePlans } from 'hooks/api/getters/useReleasePlans/useReleasePla import { ReleasePlan } from '../../../ReleasePlan/ReleasePlan'; import { StrategySeparator } from 'component/common/StrategySeparator/StrategySeparator'; import { ProjectEnvironmentStrategyDraggableItem } from './StrategyDraggableItem/ProjectEnvironmentStrategyDraggableItem'; -import { disabledStrategyClassName } from 'component/common/StrategyItemContainer/disabled-strategy-utils'; +import { + StrategyListItem, + releasePlanBackground, + strategyBackground, +} from 'component/common/StrategyList/StrategyListItem'; +import { StrategyList } from 'component/common/StrategyList/StrategyList'; interface IEnvironmentAccordionBodyProps { isDisabled: boolean; @@ -34,37 +39,6 @@ const StyledAccordionBodyInnerContainer = styled('div')(({ theme }) => ({ borderBottom: `1px solid ${theme.palette.divider}`, })); -export const StyledContentList = styled('ol')(({ theme }) => ({ - listStyle: 'none', - padding: 0, - margin: 0, - - '& > li': { - paddingBlock: theme.spacing(2.5), - position: 'relative', - }, - '& > li + li': { - borderTop: `1px solid ${theme.palette.divider}`, - }, - '& > li:first-of-type': { - paddingTop: theme.spacing(1), - }, -})); - -const releasePlanBackground = (theme: Theme) => - theme.palette.background.elevation2; -const strategyBackground = (theme: Theme) => - theme.palette.background.elevation1; - -export const StyledListItem = styled('li')<{ - 'data-type'?: 'release-plan' | 'strategy'; -}>(({ theme, ...props }) => ({ - background: - props['data-type'] === 'release-plan' - ? releasePlanBackground(theme) - : strategyBackground(theme), -})); - const AlertContainer = styled('div')(({ theme }) => ({ padding: theme.spacing(2), paddingBottom: theme.spacing(0), @@ -74,14 +48,6 @@ const AlertContainer = styled('div')(({ theme }) => ({ }, })); -// todo: consider exporting this into a shared thing or move it into the separator itself (either as a disabled prop or using the css here) -export const StyledStrategySeparator = styled(StrategySeparator)({ - [`&:has(+ * .${disabledStrategyClassName}, + .${disabledStrategyClassName})`]: - { - filter: 'grayscale(1)', - }, -}); - export const EnvironmentAccordionBody = ({ featureEnvironment, isDisabled, @@ -257,21 +223,21 @@ export const EnvironmentAccordionBody = ({ ) : null} - + {releasePlans.map((plan) => ( - + - + ))} {paginateStrategies ? ( <> {page.map((strategy, index) => ( - + {index > 0 || releasePlans.length > 0 ? ( - + ) : null} - + ))} ) : ( <> {strategies.map((strategy, index) => ( - + {index > 0 || releasePlans.length > 0 ? ( - + ) : null} - + ))} )} - + {paginateStrategies ? ( prop !== 'status', @@ -117,9 +116,9 @@ export const ReleasePlanMilestone = ({ - + {milestone.strategies.map((strategy, index) => ( - + {index > 0 ? : null} - + ))} - + ); diff --git a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/PlaygroundResultStrategyLists.tsx b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/PlaygroundResultStrategyLists.tsx index d3faab4db8..fd34c46a49 100644 --- a/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/PlaygroundResultStrategyLists.tsx +++ b/frontend/src/component/playground/Playground/PlaygroundResultsTable/FeatureResultInfoPopoverCell/FeatureStrategyList/StrategyList/PlaygroundResultStrategyLists.tsx @@ -3,12 +3,11 @@ import type { PlaygroundStrategySchema, PlaygroundRequestSchema, } from 'openapi'; -import { - StyledContentList, - StyledListItem, - StyledStrategySeparator, -} from 'component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/EnvironmentAccordionBody'; + import { FeatureStrategyItem } from './StrategyItem/FeatureStrategyItem'; +import { StrategySeparator } from 'component/common/StrategySeparator/StrategySeparator'; +import { StrategyList } from 'component/common/StrategyList/StrategyList'; +import { StrategyListItem } from 'component/common/StrategyList/StrategyListItem'; interface PlaygroundResultStrategyListProps { strategies: PlaygroundStrategySchema[]; @@ -31,7 +30,7 @@ const StyledListTitleDescription = styled('p')(({ theme }) => ({ fontSize: theme.typography.body2.fontSize, })); -const RestyledContentList = styled(StyledContentList)(({ theme }) => ({ +const StyledStrategyList = styled(StrategyList)(({ theme }) => ({ marginInline: `calc(var(--popover-inline-padding) * -1)`, borderTop: `1px solid ${theme.palette.divider}`, '> li:last-of-type': { @@ -63,17 +62,17 @@ export const PlaygroundResultStrategyLists = ({ ) : null} - + {strategies?.map((strategy, index) => ( - - {index > 0 ? : ''} + + {index > 0 ? : ''} - + ))} - + ); }; diff --git a/frontend/src/component/releases/ReleasePlanTemplate/TemplateForm/MilestoneList/MilestoneCard/MilestoneCard.tsx b/frontend/src/component/releases/ReleasePlanTemplate/TemplateForm/MilestoneList/MilestoneCard/MilestoneCard.tsx index 8b57293001..ce91f59f7a 100644 --- a/frontend/src/component/releases/ReleasePlanTemplate/TemplateForm/MilestoneList/MilestoneCard/MilestoneCard.tsx +++ b/frontend/src/component/releases/ReleasePlanTemplate/TemplateForm/MilestoneList/MilestoneCard/MilestoneCard.tsx @@ -19,15 +19,14 @@ import { ReleasePlanTemplateAddStrategyForm } from '../../MilestoneStrategy/Rele import DragIndicator from '@mui/icons-material/DragIndicator'; import { type OnMoveItem, useDragItem } from 'hooks/useDragItem'; import type { IExtendedMilestonePayload } from 'component/releases/hooks/useTemplateForm'; -import { - StyledContentList, - StyledListItem, -} from 'component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/EnvironmentAccordionBody'; + import { StrategySeparator } from 'component/common/StrategySeparator/StrategySeparator'; import Edit from '@mui/icons-material/Edit'; import Delete from '@mui/icons-material/DeleteOutlined'; import { StrategyDraggableItem } from 'component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyDraggableItem'; import { ScreenReaderOnly } from 'component/common/ScreenReaderOnly/ScreenReaderOnly'; +import { StrategyList } from 'component/common/StrategyList/StrategyList'; +import { StrategyListItem } from 'component/common/StrategyList/StrategyListItem'; const leftPadding = 3; @@ -469,9 +468,9 @@ export const MilestoneCard = ({ /> - + {milestone.strategies.map((strg, index) => ( - + {index > 0 ? : null} } /> - + ))} - +