From e2bb894f6805e0eef6a78697c68460f43b9ccdc8 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Mon, 30 Jun 2025 11:53:21 +0200 Subject: [PATCH] feat(1-3878)/diffable segment changes (#10234) Adds change / view diff tab buttons to segment changes too. Extracts the tab definitions and stylings into its own file so that it's easier to share across CR change components. Moves the old segment change details into the legacy segment change file. Change views: image Diff views: image --- .../Changes/Change/ChangeTabComponents.tsx | 54 +++++++ .../Change/LegacySegmentChangeDetails.tsx | 129 +++++++++++++++++ .../Changes/Change/SegmentChange.tsx | 8 +- .../Changes/Change/SegmentChangeDetails.tsx | 132 +++++++++++------- .../Changes/Change/StrategyChange.tsx | 78 ++--------- .../ChangeRequest/SegmentTooltipLink.tsx | 23 ++- .../StrategyTooltipLink.tsx | 36 +++-- .../component/events/EventDiff/EventDiff.tsx | 2 +- 8 files changed, 319 insertions(+), 143 deletions(-) create mode 100644 frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeTabComponents.tsx create mode 100644 frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacySegmentChangeDetails.tsx diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeTabComponents.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeTabComponents.tsx new file mode 100644 index 0000000000..76807f37e0 --- /dev/null +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeTabComponents.tsx @@ -0,0 +1,54 @@ +import { + Tab as MuiTab, + TabPanel as MuiTabPanel, + Tabs as MuiTabs, + TabsList as MuiTabsList, +} from '@mui/base'; +import { Button, type ButtonProps, styled } from '@mui/material'; +import type { PropsWithChildren } from 'react'; + +export const TabList = styled(MuiTabsList)(({ theme }) => ({ + display: 'inline-flex', + flexDirection: 'row', + gap: theme.spacing(0.5), +})); + +const StyledButton = styled(Button)(({ theme }) => ({ + whiteSpace: 'nowrap', + color: theme.palette.text.secondary, + fontWeight: 'normal', + '&[aria-selected="true"]': { + fontWeight: 'bold', + color: theme.palette.primary.main, + background: theme.palette.background.elevation1, + }, +})); + +export const Tab = styled(({ children }: ButtonProps) => ( + {children} +))(({ theme }) => ({ + position: 'absolute', + top: theme.spacing(-0.5), + left: theme.spacing(2), + transform: 'translateY(-50%)', + padding: theme.spacing(0.75, 1), + lineHeight: 1, + fontSize: theme.fontSizes.smallerBody, + color: theme.palette.text.primary, + background: theme.palette.background.application, + borderRadius: theme.shape.borderRadiusExtraLarge, + zIndex: theme.zIndex.fab, + textTransform: 'uppercase', +})); + +export const Tabs = ({ children }: PropsWithChildren) => ( + + {children} + +); + +export const TabPanel = MuiTabPanel; diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacySegmentChangeDetails.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacySegmentChangeDetails.tsx new file mode 100644 index 0000000000..ca516d38a8 --- /dev/null +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacySegmentChangeDetails.tsx @@ -0,0 +1,129 @@ +import type React from 'react'; +import type { FC, ReactNode } from 'react'; +import { Box, styled, Typography } from '@mui/material'; +import type { + ChangeRequestState, + IChangeRequestDeleteSegment, + IChangeRequestUpdateSegment, +} from 'component/changeRequest/changeRequest.types'; +import { useSegment } from 'hooks/api/getters/useSegment/useSegment'; +import { SegmentDiff, SegmentTooltipLink } from '../../SegmentTooltipLink.tsx'; + +import { ViewableConstraintsList } from 'component/common/NewConstraintAccordion/ConstraintsList/ViewableConstraintsList'; + +import { ChangeOverwriteWarning } from './ChangeOverwriteWarning/ChangeOverwriteWarning.tsx'; + +const ChangeItemCreateEditWrapper = styled(Box)(({ theme }) => ({ + display: 'grid', + gridTemplateColumns: 'auto 40px', + gap: theme.spacing(1), + alignItems: 'center', + width: '100%', + margin: theme.spacing(0, 0, 1, 0), +})); + +export const ChangeItemWrapper = styled(Box)({ + display: 'flex', + justifyContent: 'space-between', + alignItems: 'center', +}); + +const ChangeItemInfo: FC<{ children?: React.ReactNode }> = styled(Box)( + ({ theme }) => ({ + display: 'grid', + gridTemplateColumns: '150px auto', + gridAutoFlow: 'column', + alignItems: 'center', + flexGrow: 1, + gap: theme.spacing(1), + }), +); + +const SegmentContainer = styled(Box, { + shouldForwardProp: (prop) => prop !== 'conflict', +})<{ conflict: string | undefined }>(({ theme, conflict }) => ({ + borderLeft: '1px solid', + borderRight: '1px solid', + borderTop: '1px solid', + borderBottom: '1px solid', + borderColor: conflict + ? theme.palette.warning.border + : theme.palette.divider, + borderTopColor: theme.palette.divider, + padding: theme.spacing(3), + borderRadius: `0 0 ${theme.shape.borderRadiusLarge}px ${theme.shape.borderRadiusLarge}px`, +})); + +/** + * Deprecated: use SegmentChangeDetails instead. Remove file with flag crDiffView + * @deprecated + */ +export const LegacySegmentChangeDetails: FC<{ + actions?: ReactNode; + change: IChangeRequestUpdateSegment | IChangeRequestDeleteSegment; + changeRequestState: ChangeRequestState; +}> = ({ actions, change, changeRequestState }) => { + const { segment: currentSegment } = useSegment(change.payload.id); + const snapshotSegment = change.payload.snapshot; + const previousName = + changeRequestState === 'Applied' + ? change.payload?.snapshot?.name + : currentSegment?.name; + const referenceSegment = + changeRequestState === 'Applied' ? snapshotSegment : currentSegment; + + return ( + + {change.action === 'deleteSegment' && ( + + + ({ + color: theme.palette.error.main, + })} + > + - Deleting segment: + + + + + +
{actions}
+ + )} + {change.action === 'updateSegment' && ( + <> + + + + Editing segment: + + + + +
{actions}
+ + + + )} + + ); +}; diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChange.tsx index ad3bb088b0..3a9e5517c4 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChange.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChange.tsx @@ -5,9 +5,11 @@ import type { ChangeRequestState, ISegmentChange, } from '../../../changeRequest.types'; +import { LegacySegmentChangeDetails } from './LegacySegmentChangeDetails.tsx'; import { SegmentChangeDetails } from './SegmentChangeDetails.tsx'; import { ConflictWarning } from './ConflictWarning.tsx'; import { useSegment } from 'hooks/api/getters/useSegment/useSegment.ts'; +import { useUiFlag } from 'hooks/useUiFlag.ts'; interface ISegmentChangeProps { segmentChange: ISegmentChange; @@ -24,6 +26,10 @@ export const SegmentChange: FC = ({ }) => { const { segment } = useSegment(segmentChange.payload.id); + const ChangeDetails = useUiFlag('crDiffView') + ? SegmentChangeDetails + : LegacySegmentChangeDetails; + return ( = ({ - ({ - display: 'grid', - gridTemplateColumns: 'auto 40px', + display: 'flex', gap: theme.spacing(1), alignItems: 'center', width: '100%', @@ -68,58 +67,89 @@ export const SegmentChangeDetails: FC<{ const referenceSegment = changeRequestState === 'Applied' ? snapshotSegment : currentSegment; + const actionsWithTabs = ( + <> + + Change + View diff + + {actions} + + ); + return ( - - {change.action === 'deleteSegment' && ( - - - ({ - color: theme.palette.error.main, - })} - > - - Deleting segment: - - + + + {change.action === 'deleteSegment' && ( + <> + + + ({ + color: theme.palette.error.main, + })} + > + - Deleting segment: + + + + + + {actionsWithTabs} + + + + - - -
{actions}
- - )} - {change.action === 'updateSegment' && ( - <> - - - - Editing segment: - - - - -
{actions}
- - - - )} - + + + )} + {change.action === 'updateSegment' && ( + <> + + + + Editing segment: + + + + + {actionsWithTabs} + + + + + + + + + + )} + + ); }; diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx index 639e06672e..988ea09943 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx @@ -1,14 +1,6 @@ import type React from 'react'; import type { FC, ReactNode } from 'react'; -import { - Box, - Button, - type ButtonProps, - styled, - Tooltip, - Typography, -} from '@mui/material'; -import { Tab, Tabs, TabsList, TabPanel } from '@mui/base'; +import { Box, styled, Tooltip, Typography } from '@mui/material'; import BlockIcon from '@mui/icons-material/Block'; import TrackChangesIcon from '@mui/icons-material/TrackChanges'; import { @@ -29,6 +21,7 @@ import { flexRow } from 'themes/themeStyles'; import { EnvironmentVariantsTable } from 'component/feature/FeatureView/FeatureVariants/FeatureEnvironmentVariants/EnvironmentVariantsCard/EnvironmentVariantsTable/EnvironmentVariantsTable'; import { ChangeOverwriteWarning } from './ChangeOverwriteWarning/ChangeOverwriteWarning.tsx'; import type { IFeatureStrategy } from 'interfaces/strategy'; +import { Tab, TabList, TabPanel, Tabs } from './ChangeTabComponents.tsx'; export const ChangeItemWrapper = styled(Box)({ display: 'flex', @@ -41,6 +34,7 @@ const ChangeItemCreateEditDeleteWrapper = styled(Box)(({ theme }) => ({ justifyContent: 'space-between', gap: theme.spacing(1), alignItems: 'center', + marginBottom: theme.spacing(1), width: '100%', })); @@ -180,46 +174,6 @@ const DeleteStrategy: FC<{ ); }; -const ActionsContainer = styled('div')(({ theme }) => ({ - display: 'flex', - gap: theme.spacing(1), - alignItems: 'center', -})); - -const StyledTabList = styled(TabsList)(({ theme }) => ({ - display: 'inline-flex', - flexDirection: 'row', - gap: theme.spacing(0.5), -})); - -const StyledButton = styled(Button)(({ theme }) => ({ - whiteSpace: 'nowrap', - color: theme.palette.text.secondary, - fontWeight: 'normal', - '&[aria-selected="true"]': { - fontWeight: 'bold', - color: theme.palette.primary.main, - background: theme.palette.background.elevation1, - }, -})); - -export const StyledTab = styled(({ children }: ButtonProps) => ( - {children} -))(({ theme }) => ({ - position: 'absolute', - top: theme.spacing(-0.5), - left: theme.spacing(2), - transform: 'translateY(-50%)', - padding: theme.spacing(0.75, 1), - lineHeight: 1, - fontSize: theme.fontSizes.smallerBody, - color: theme.palette.text.primary, - background: theme.palette.background.application, - borderRadius: theme.shape.borderRadiusExtraLarge, - zIndex: theme.zIndex.fab, - textTransform: 'uppercase', -})); - const UpdateStrategy: FC<{ change: IChangeRequestUpdateStrategy; changeRequestState: ChangeRequestState; @@ -397,31 +351,27 @@ export const StrategyChange: FC<{ environmentName, ); - const Actions = ( - - - Change - View diff - + const actionsWithTabs = ( + <> + + Change + View diff + {actions} - + ); return ( - + {change.action === 'addStrategy' && ( - + )} {change.action === 'deleteStrategy' && ( )} {change.action === 'updateStrategy' && ( @@ -429,7 +379,7 @@ export const StrategyChange: FC<{ change={change} changeRequestState={changeRequestState} currentStrategy={currentStrategy} - actions={Actions} + actions={actionsWithTabs} /> )} diff --git a/frontend/src/component/changeRequest/ChangeRequest/SegmentTooltipLink.tsx b/frontend/src/component/changeRequest/ChangeRequest/SegmentTooltipLink.tsx index 30ee1926b1..50670c1333 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/SegmentTooltipLink.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/SegmentTooltipLink.tsx @@ -3,14 +3,15 @@ import type { IChangeRequestUpdateSegment, } from 'component/changeRequest/changeRequest.types'; import type React from 'react'; -import type { FC } from 'react'; -import EventDiff from 'component/events/EventDiff/EventDiff'; +import { Fragment, type FC } from 'react'; import omit from 'lodash.omit'; import { TooltipLink } from 'component/common/TooltipLink/TooltipLink'; import { styled } from '@mui/material'; import { textTruncated } from 'themes/themeStyles'; import type { ISegment } from 'interfaces/segment'; import { NameWithChangeInfo } from './NameWithChangeInfo/NameWithChangeInfo.tsx'; +import { EventDiff } from 'component/events/EventDiff/EventDiff.tsx'; +import { useUiFlag } from 'hooks/useUiFlag.ts'; const StyledCodeSection = styled('div')(({ theme }) => ({ overflowX: 'auto', @@ -23,22 +24,32 @@ const StyledCodeSection = styled('div')(({ theme }) => ({ }, })); +const omitIfDefined = (obj: any, keys: string[]) => + obj ? omit(obj, keys) : obj; + export const SegmentDiff: FC<{ change: IChangeRequestUpdateSegment | IChangeRequestDeleteSegment; currentSegment?: ISegment; }> = ({ change, currentSegment }) => { + const useNewDiff = useUiFlag('improvedJsonDiff'); + const Wrapper = useNewDiff ? Fragment : StyledCodeSection; + const omissionFunction = useNewDiff ? omitIfDefined : omit; + const changeRequestSegment = change.action === 'deleteSegment' ? undefined : change.payload; return ( - + - + ); }; interface IStrategyTooltipLinkProps { diff --git a/frontend/src/component/changeRequest/ChangeRequest/StrategyTooltipLink/StrategyTooltipLink.tsx b/frontend/src/component/changeRequest/ChangeRequest/StrategyTooltipLink/StrategyTooltipLink.tsx index d6f7dd15c1..f3d5b4248f 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/StrategyTooltipLink/StrategyTooltipLink.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/StrategyTooltipLink/StrategyTooltipLink.tsx @@ -4,12 +4,12 @@ import type { IChangeRequestUpdateStrategy, } from 'component/changeRequest/changeRequest.types'; import type React from 'react'; -import type { FC } from 'react'; +import { Fragment, type FC } from 'react'; import { formatStrategyName, GetFeatureStrategyIcon, } from 'utils/strategyNames'; -import EventDiff, { NewEventDiff } from 'component/events/EventDiff/EventDiff'; +import { EventDiff } from 'component/events/EventDiff/EventDiff'; import omit from 'lodash.omit'; import { TooltipLink } from 'component/common/TooltipLink/TooltipLink'; import { Typography, styled } from '@mui/material'; @@ -41,6 +41,9 @@ const sortSegments = ( }; }; +const omitIfDefined = (obj: any, keys: string[]) => + obj ? omit(obj, keys) : obj; + export const StrategyDiff: FC<{ change: | IChangeRequestAddStrategy @@ -54,29 +57,22 @@ export const StrategyDiff: FC<{ const sortedCurrentStrategy = sortSegments(currentStrategy); const sortedChangeRequestStrategy = sortSegments(changeRequestStrategy); - if (useNewDiff) { - return ( - - ); - } + + const Wrapper = useNewDiff ? Fragment : StyledCodeSection; + const omissionFunction = useNewDiff ? omitIfDefined : omit; return ( - + - + ); }; diff --git a/frontend/src/component/events/EventDiff/EventDiff.tsx b/frontend/src/component/events/EventDiff/EventDiff.tsx index 93968bc36f..0e43014421 100644 --- a/frontend/src/component/events/EventDiff/EventDiff.tsx +++ b/frontend/src/component/events/EventDiff/EventDiff.tsx @@ -212,7 +212,7 @@ const OldEventDiff: FC = ({ ); }; -const EventDiff: FC = (props) => { +export const EventDiff: FC = (props) => { const useNewJsonDiff = useUiFlag('improvedJsonDiff'); if (useNewJsonDiff) { return ;