From bb473001c0cf09dcb67ec5616ca80a707ea4c4b2 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Wed, 2 Jul 2025 09:42:59 +0200 Subject: [PATCH] chore: remove view diff links in new components + remove colons (#10262) Removes the view diff hover links (and strategy icons) in the new views and removes trailing colons. In removing the hover links, I have split up the content of their files (`StrategyTooltipLink` and `SegmentTooltipLink`) into `Change{Segment|Strategy}Name` and `{Segment|Strategy}Diff`. I have reverted the existing tooltip files to their state before I began changing this and added deprecation notices. These old tooltips are only used by the old components, so we don't need to work on them after all. In doing this work, I've also updated the strategy change diff to handle the new functionality (tabs instead of hover) The removal of the trailing colons (so that it's `adding strategy gradual rollout` instead of `adding strategy: gradual rollout` is in preparation for the remaining changes to the header that we're introducing with this change. The removal of these is not behind a flag, so I've also done it in the legacy components. This feels like a very low risk change, so it felt like more work to have to check a flag for each of the different instances where we use it. image --- .../Changes/Change/Change.styles.tsx | 9 ++ .../Changes/Change/ChangeSegmentName.tsx | 24 +++++ .../Changes/Change/ChangeStrategyName.tsx | 30 ++++++ .../Changes/Change/DependencyChange.tsx | 2 +- .../EnvironmentStrategyExecutionOrder.tsx | 4 +- .../Change/LegacyReleasePlanChange.tsx | 6 +- .../Change/LegacySegmentChangeDetails.tsx | 4 +- .../Changes/Change/LegacyStrategyChange.tsx | 12 +-- .../NameWithChangeInfo.test.tsx | 0 .../NameWithChangeInfo/NameWithChangeInfo.tsx | 0 .../Changes/Change/ReleasePlanChange.tsx | 33 +----- .../Changes/Change/SegmentChangeDetails.tsx | 36 ++----- .../Changes/Change/SegmentDiff.tsx | 31 ++++++ .../Changes/Change/StrategyChange.test.tsx | 102 ++++++++++++------ .../Changes/Change/StrategyChange.tsx | 66 ++++-------- .../Changes/Change/StrategyDiff.tsx | 47 ++++++++ .../Changes/Change/ToggleStatusChange.tsx | 2 +- .../ChangeRequest/SegmentTooltipLink.tsx | 26 ++--- .../StrategyTooltipLink.tsx | 24 ++--- 19 files changed, 273 insertions(+), 185 deletions(-) create mode 100644 frontend/src/component/changeRequest/ChangeRequest/Changes/Change/Change.styles.tsx create mode 100644 frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeSegmentName.tsx create mode 100644 frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeStrategyName.tsx rename frontend/src/component/changeRequest/ChangeRequest/{ => Changes/Change}/NameWithChangeInfo/NameWithChangeInfo.test.tsx (100%) rename frontend/src/component/changeRequest/ChangeRequest/{ => Changes/Change}/NameWithChangeInfo/NameWithChangeInfo.tsx (100%) create mode 100644 frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentDiff.tsx create mode 100644 frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyDiff.tsx diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/Change.styles.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/Change.styles.tsx new file mode 100644 index 0000000000..bb629e269f --- /dev/null +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/Change.styles.tsx @@ -0,0 +1,9 @@ +import { styled, Typography } from '@mui/material'; + +export const ChangeItemInfo = styled(Typography)(({ theme }) => ({ + display: 'flex', + flexFlow: 'row', + alignItems: 'center', + flex: 'auto', + gap: `1ch`, +})); diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeSegmentName.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeSegmentName.tsx new file mode 100644 index 0000000000..f039f8803e --- /dev/null +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeSegmentName.tsx @@ -0,0 +1,24 @@ +import type { FC } from 'react'; +import { styled } from '@mui/material'; +import { textTruncated } from 'themes/themeStyles'; +import { NameWithChangeInfo } from './NameWithChangeInfo/NameWithChangeInfo.tsx'; + +type ChangeSegmentNameProps = { + name?: string; + previousName?: string; +}; + +const Truncated = styled('div')(() => ({ + ...textTruncated, + maxWidth: 500, + display: 'flex', +})); + +export const ChangeSegmentName: FC = ({ + name, + previousName, +}) => ( + + + +); diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeStrategyName.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeStrategyName.tsx new file mode 100644 index 0000000000..20eb1fca20 --- /dev/null +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeStrategyName.tsx @@ -0,0 +1,30 @@ +import type { FC } from 'react'; +import { formatStrategyName } from 'utils/strategyNames'; +import { styled } from '@mui/material'; +import { textTruncated } from 'themes/themeStyles'; +import { NameWithChangeInfo } from './NameWithChangeInfo/NameWithChangeInfo.tsx'; +import { Truncator } from 'component/common/Truncator/Truncator.tsx'; + +type ChangeStrategyNameProps = { + name: string; + title?: string; + previousTitle?: string; +}; + +const Truncated = styled('span')(() => ({ + ...textTruncated, + maxWidth: 500, +})); + +export const ChangeStrategyName: FC = ({ + name, + title, + previousTitle, +}) => { + return ( + + {formatStrategyName(name)} + + + ); +}; diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/DependencyChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/DependencyChange.tsx index 445975fcf8..3320499c29 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/DependencyChange.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/DependencyChange.tsx @@ -34,7 +34,7 @@ export const DependencyChange: VFC<{ - + Adding dependency: + + Adding dependency -

Updating strategy execution order to:

+

Updating strategy execution order to

Change @@ -140,7 +140,7 @@ export const EnvironmentStrategyExecutionOrder = ({ maxHeight: 600, }} > - Updating strategy execution order to: + Updating strategy execution order to {actions} diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacyReleasePlanChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacyReleasePlanChange.tsx index b087d1cbfb..2a819076f2 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacyReleasePlanChange.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacyReleasePlanChange.tsx @@ -76,7 +76,7 @@ const DeleteReleasePlan: FC<{ color: theme.palette.error.main, })} > - - Deleting release plan: + - Deleting release plan {releasePlan.name} @@ -219,11 +219,11 @@ const AddReleasePlan: FC<{ current {' '} - release plan with: + release plan with ) : ( - + Adding release plan: + + Adding release plan )} {planPreview.name} diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacySegmentChangeDetails.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacySegmentChangeDetails.tsx index ca516d38a8..11de3e5882 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacySegmentChangeDetails.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacySegmentChangeDetails.tsx @@ -82,7 +82,7 @@ export const LegacySegmentChangeDetails: FC<{ color: theme.palette.error.main, })} > - - Deleting segment: + - Deleting segment - Editing segment: + Editing segment = ({ wasDisabled = false, willBeDisabled = false }) => { if (wasDisabled && willBeDisabled) { return ( - Editing strategy: + Editing strategy ); } if (!wasDisabled && willBeDisabled) { - return Editing strategy:; + return Editing strategy; } if (wasDisabled && !willBeDisabled) { - return Editing strategy:; + return Editing strategy; } - return Editing strategy:; + return Editing strategy; }; const hasDiff = (object: unknown, objectToCompare: unknown) => @@ -148,7 +148,7 @@ const DeleteStrategy: FC<{ color: theme.palette.error.main, })} > - - Deleting strategy: + - Deleting strategy - + Adding strategy: + + Adding strategy ({ width: '100%', })); -const ChangeItemInfo: FC<{ children?: React.ReactNode }> = styled(Box)( - ({ theme }) => ({ - display: 'flex', - gap: theme.spacing(1), - }), -); - -const ViewDiff = styled('span')(({ theme }) => ({ - color: theme.palette.primary.main, - marginLeft: theme.spacing(1), -})); - -const StyledCodeSection = styled('div')(({ theme }) => ({ - overflowX: 'auto', - '& code': { - wordWrap: 'break-word', - whiteSpace: 'pre-wrap', - fontFamily: 'monospace', - lineHeight: 1.5, - fontSize: theme.fontSizes.smallBody, - }, -})); - const DeleteReleasePlan: FC<{ change: IChangeRequestDeleteReleasePlan; currentReleasePlan?: IReleasePlan; @@ -77,7 +54,7 @@ const DeleteReleasePlan: FC<{ color: theme.palette.error.main, })} > - - Deleting release plan: + - Deleting release plan {releasePlan.name} @@ -116,7 +93,7 @@ const StartMilestone: FC<{ - + Start milestone: + + Start milestone {newMilestone.name} @@ -188,7 +165,7 @@ const AddReleasePlan: FC<{ - + Adding release plan: + + Adding release plan {planPreview.name} @@ -230,7 +207,7 @@ const AddReleasePlan: FC<{ current {' '} - release plan with: + release plan with {planPreview.name} diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChangeDetails.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChangeDetails.tsx index fa7c6d0e41..cae90a8771 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChangeDetails.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChangeDetails.tsx @@ -1,4 +1,3 @@ -import type React from 'react'; import type { FC, ReactNode } from 'react'; import { Box, styled, Typography } from '@mui/material'; import type { @@ -7,11 +6,13 @@ import type { 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'; import { Tab, TabList, TabPanel, Tabs } from './ChangeTabComponents.tsx'; +import { ChangeItemInfo } from './Change.styles.tsx'; +import { ChangeSegmentName } from './ChangeSegmentName.tsx'; +import { SegmentDiff } from './SegmentDiff.tsx'; const ChangeItemCreateEditWrapper = styled(Box)(({ theme }) => ({ display: 'flex', @@ -27,17 +28,6 @@ export const ChangeItemWrapper = styled(Box)({ 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 }) => ({ @@ -89,17 +79,12 @@ export const SegmentChangeDetails: FC<{ color: theme.palette.error.main, })} > - - Deleting segment: + - Deleting segment - - - + /> {actionsWithTabs} @@ -125,13 +110,8 @@ export const SegmentChangeDetails: FC<{ /> - Editing segment: - - - + Editing segment + {actionsWithTabs} diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentDiff.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentDiff.tsx new file mode 100644 index 0000000000..8c3074c311 --- /dev/null +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentDiff.tsx @@ -0,0 +1,31 @@ +import type { + IChangeRequestDeleteSegment, + IChangeRequestUpdateSegment, +} from 'component/changeRequest/changeRequest.types'; +import type { FC } from 'react'; +import omit from 'lodash.omit'; +import type { ISegment } from 'interfaces/segment'; +import { EventDiff } from 'component/events/EventDiff/EventDiff.tsx'; + +const omitIfDefined = (obj: any, keys: string[]) => + obj ? omit(obj, keys) : obj; + +export const SegmentDiff: FC<{ + change: IChangeRequestUpdateSegment | IChangeRequestDeleteSegment; + currentSegment?: ISegment; +}> = ({ change, currentSegment }) => { + const changeRequestSegment = + change.action === 'deleteSegment' ? undefined : change.payload; + + return ( + + ); +}; diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.test.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.test.tsx index 8ee207f292..514baf1c7d 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.test.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.test.tsx @@ -27,6 +27,12 @@ const strategy = { }; const setupApi = () => { + testServerRoute(server, '/api/admin/ui-config', { + flags: { + improvedJsonDiff: true, + }, + }); + testServerRoute( server, `/api/admin/projects/${projectId}/features/${feature}`, @@ -107,20 +113,26 @@ test('Editing strategy before change request is applied diffs against current st { route: `/projects/${projectId}` }, ); - await screen.findByText('Editing strategy:'); + await screen.findByText('Editing strategy'); await screen.findByText('change_request_title'); await screen.findByText('current_title'); expect(screen.queryByText('snapshot_title')).not.toBeInTheDocument(); - const viewDiff = await screen.findByText('View Diff'); - await userEvent.hover(viewDiff); - await screen.findByText(`- parameters.rollout: "${currentRollout}"`); - await screen.findByText('- variants.0.name: "current_variant"'); - await screen.findByText('+ variants.0.name: "change_variant"'); - await screen.findByText('Updating strategy variants to:'); const variants = await screen.findAllByText('change_variant'); expect(variants).toHaveLength(2); + + const viewDiff = await screen.findByRole('tab', { + name: 'View diff', + }); + await userEvent.click(viewDiff); + + const rollout = await screen.findByText(`rollout: "${currentRollout}"`); + expect(rollout).toHaveClass('deletion'); + const oldName = await screen.findByText('name: "current_variant"'); + expect(oldName).toHaveClass('deletion'); + const newName = await screen.findByText('name: "change_variant"'); + expect(newName).toHaveClass('addition'); }); test('Editing strategy after change request is applied diffs against the snapshot', async () => { @@ -177,21 +189,31 @@ test('Editing strategy after change request is applied diffs against the snapsho { route: `/projects/${projectId}` }, ); - await screen.findByText('Editing strategy:'); + await screen.findByText('Editing strategy'); await screen.findByText('change_request_title'); await screen.findByText('snapshot_title'); expect(screen.queryByText('current_title')).not.toBeInTheDocument(); - const viewDiff = await screen.findByText('View Diff'); - await userEvent.hover(viewDiff); - await screen.findByText(`- parameters.rollout: "${snapshotRollout}"`); - await screen.findByText(`+ parameters.rollout: "${changeRequestRollout}"`); - await screen.findByText('- variants.0.name: "snapshot_variant"'); - await screen.findByText('+ variants.0.name: "change_variant"'); - await screen.findByText('Updating strategy variants to:'); const variants = await screen.findAllByText('change_variant'); expect(variants).toHaveLength(2); + + const viewDiff = await screen.findByRole('tab', { + name: 'View diff', + }); + await userEvent.click(viewDiff); + + const oldRollout = await screen.findByText(`rollout: "${snapshotRollout}"`); + expect(oldRollout).toHaveClass('deletion'); + const newRollout = await screen.findByText( + `rollout: "${changeRequestRollout}"`, + ); + + expect(newRollout).toHaveClass('addition'); + const oldName = await screen.findByText('name: "snapshot_variant"'); + expect(oldName).toHaveClass('deletion'); + const newName = await screen.findByText('name: "change_variant"'); + expect(newName).toHaveClass('addition'); }); test('Deleting strategy before change request is applied diffs against current strategy', async () => { @@ -220,15 +242,18 @@ test('Deleting strategy before change request is applied diffs against current s { route: `/projects/${projectId}` }, ); - await screen.findByText('- Deleting strategy:'); + await screen.findByText('- Deleting strategy'); await screen.findByText('Gradual rollout'); await screen.findByText('current_title'); - const viewDiff = await screen.findByText('View Diff'); - await userEvent.hover(viewDiff); - await screen.findByText('- constraints (deleted)'); - await screen.findByText('current_variant'); + + const viewDiff = await screen.findByRole('tab', { + name: 'View diff', + }); + await userEvent.click(viewDiff); + const element = await screen.findByText('name: "flexibleRollout"'); + expect(element).toHaveClass('deletion'); }); test('Deleting strategy after change request is applied diffs against the snapshot', async () => { @@ -273,16 +298,20 @@ test('Deleting strategy after change request is applied diffs against the snapsh { route: `/projects/${projectId}` }, ); - await screen.findByText('- Deleting strategy:'); + await screen.findByText('- Deleting strategy'); await screen.findByText('Gradual rollout'); await screen.findByText('snapshot_title'); expect(screen.queryByText('current_title')).not.toBeInTheDocument(); - const viewDiff = await screen.findByText('View Diff'); - await userEvent.hover(viewDiff); - await screen.findByText('- constraints (deleted)'); - await screen.findByText('snapshot_variant'); + + const viewDiff = await screen.findByRole('tab', { + name: 'View diff', + }); + await userEvent.click(viewDiff); + + const element = await screen.findByText('name: "snapshot_variant"'); + expect(element).toHaveClass('deletion'); }); test('Adding strategy always diffs against undefined strategy', async () => { @@ -323,14 +352,19 @@ test('Adding strategy always diffs against undefined strategy', async () => { { route: `/projects/${projectId}` }, ); - await screen.findByText('+ Adding strategy:'); + await screen.findByText('+ Adding strategy'); await screen.findByText('change_request_title'); - const viewDiff = await screen.findByText('View Diff'); - await userEvent.hover(viewDiff); - await screen.findByText(`+ name: "flexibleRollout"`); const variants = await screen.findAllByText('change_variant'); expect(variants).toHaveLength(2); + + const viewDiff = await screen.findByRole('tab', { + name: 'View diff', + }); + await userEvent.click(viewDiff); + + const element = await screen.findByText('name: "flexibleRollout"'); + expect(element).toHaveClass('addition'); }); test('Segments order does not matter for diff calculation', async () => { @@ -363,7 +397,11 @@ test('Segments order does not matter for diff calculation', async () => { { route: `/projects/${projectId}` }, ); - const viewDiff = await screen.findByText('View Diff'); - await userEvent.hover(viewDiff); - await screen.findByText('(no changes)'); + const viewDiff = await screen.findByRole('tab', { + name: 'View diff', + }); + await userEvent.click(viewDiff); + + const segmentsChangeElement = screen.queryByText('segments: ['); + expect(segmentsChangeElement).not.toBeInTheDocument(); }); diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx index d5cc72eea3..3e5eecc465 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx @@ -3,10 +3,6 @@ import type { FC, ReactNode } from 'react'; import { Box, styled, Tooltip, Typography } from '@mui/material'; import BlockIcon from '@mui/icons-material/Block'; import TrackChangesIcon from '@mui/icons-material/TrackChanges'; -import { - StrategyDiff, - StrategyTooltipLink, -} from '../../StrategyTooltipLink/StrategyTooltipLink.tsx'; import { StrategyExecution } from 'component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution'; import type { ChangeRequestState, @@ -22,6 +18,9 @@ import { EnvironmentVariantsTable } from 'component/feature/FeatureView/FeatureV import { ChangeOverwriteWarning } from './ChangeOverwriteWarning/ChangeOverwriteWarning.tsx'; import type { IFeatureStrategy } from 'interfaces/strategy'; import { Tab, TabList, TabPanel, Tabs } from './ChangeTabComponents.tsx'; +import { ChangeStrategyName } from './ChangeStrategyName.tsx'; +import { StrategyDiff } from './StrategyDiff.tsx'; +import { ChangeItemInfo } from './Change.styles.tsx'; export const ChangeItemWrapper = styled(Box)({ display: 'flex', @@ -38,17 +37,6 @@ const ChangeItemCreateEditDeleteWrapper = styled(Box)(({ theme }) => ({ width: '100%', })); -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 StyledBox: FC<{ children?: React.ReactNode }> = styled(Box)( ({ theme }) => ({ marginTop: theme.spacing(2), @@ -101,20 +89,18 @@ const EditHeader: FC<{ willBeDisabled?: boolean; }> = ({ wasDisabled = false, willBeDisabled = false }) => { if (wasDisabled && willBeDisabled) { - return ( - Editing strategy: - ); + return Editing strategy; } if (!wasDisabled && willBeDisabled) { - return Editing strategy:; + return Editing strategy; } if (wasDisabled && !willBeDisabled) { - return Editing strategy:; + return Editing strategy; } - return Editing strategy:; + return Editing strategy; }; const hasDiff = (object: unknown, objectToCompare: unknown) => @@ -148,14 +134,9 @@ const DeleteStrategy: FC<{ color: theme.palette.error.main, })} > - - Deleting strategy: + - Deleting strategy - - - + {actions} @@ -209,16 +190,11 @@ const UpdateStrategy: FC<{ wasDisabled={currentStrategy?.disabled} willBeDisabled={change.payload?.disabled} /> - - - + /> {actions} @@ -286,24 +262,20 @@ const AddStrategy: FC<{ - + Adding strategy: + + Adding strategy - - - -
- -
+ /> + {actions} diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyDiff.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyDiff.tsx new file mode 100644 index 0000000000..c19c354e80 --- /dev/null +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyDiff.tsx @@ -0,0 +1,47 @@ +import type { + IChangeRequestAddStrategy, + IChangeRequestDeleteStrategy, + IChangeRequestUpdateStrategy, +} from 'component/changeRequest/changeRequest.types'; +import type { FC } from 'react'; +import { EventDiff } from 'component/events/EventDiff/EventDiff'; +import omit from 'lodash.omit'; +import type { IFeatureStrategy } from 'interfaces/strategy'; + +const sortSegments = ( + item?: T, +): T | undefined => { + if (!item || !item.segments) { + return item; + } + return { + ...item, + segments: [...item.segments].sort((a, b) => a - b), + }; +}; + +const omitIfDefined = (obj: any, keys: string[]) => + obj ? omit(obj, keys) : obj; + +export const StrategyDiff: FC<{ + change: + | IChangeRequestAddStrategy + | IChangeRequestUpdateStrategy + | IChangeRequestDeleteStrategy; + currentStrategy?: IFeatureStrategy; +}> = ({ change, currentStrategy }) => { + const changeRequestStrategy = + change.action === 'deleteStrategy' ? undefined : change.payload; + + const sortedCurrentStrategy = sortSegments(currentStrategy); + const sortedChangeRequestStrategy = sortSegments(changeRequestStrategy); + + return ( + + ); +}; diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ToggleStatusChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ToggleStatusChange.tsx index f76dc7159f..3ad51db29f 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ToggleStatusChange.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ToggleStatusChange.tsx @@ -15,7 +15,7 @@ export const ToggleStatusChange: VFC = ({ return ( - New status:{' '} + New status ({ marginLeft: theme.spacing(1) })} color={enabled ? 'success' : 'error'} diff --git a/frontend/src/component/changeRequest/ChangeRequest/SegmentTooltipLink.tsx b/frontend/src/component/changeRequest/ChangeRequest/SegmentTooltipLink.tsx index 50670c1333..c569d9f5a1 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/SegmentTooltipLink.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/SegmentTooltipLink.tsx @@ -1,17 +1,17 @@ +// deprecated: remove with flag crDiffView import type { IChangeRequestDeleteSegment, IChangeRequestUpdateSegment, } from 'component/changeRequest/changeRequest.types'; import type React from 'react'; -import { Fragment, type FC } from 'react'; +import type { FC } from 'react'; +import { EventDiff } from 'component/events/EventDiff/EventDiff'; 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'; +import { NameWithChangeInfo } from './Changes/Change/NameWithChangeInfo/NameWithChangeInfo.tsx'; const StyledCodeSection = styled('div')(({ theme }) => ({ overflowX: 'auto', @@ -24,32 +24,22 @@ 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 f3d5b4248f..ca1a64c50b 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/StrategyTooltipLink/StrategyTooltipLink.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/StrategyTooltipLink/StrategyTooltipLink.tsx @@ -1,10 +1,11 @@ +// deprecated: remove with flag crDiffView import type { IChangeRequestAddStrategy, IChangeRequestDeleteStrategy, IChangeRequestUpdateStrategy, } from 'component/changeRequest/changeRequest.types'; import type React from 'react'; -import { Fragment, type FC } from 'react'; +import type { FC } from 'react'; import { formatStrategyName, GetFeatureStrategyIcon, @@ -15,8 +16,7 @@ import { TooltipLink } from 'component/common/TooltipLink/TooltipLink'; import { Typography, styled } from '@mui/material'; import type { IFeatureStrategy } from 'interfaces/strategy'; import { textTruncated } from 'themes/themeStyles'; -import { NameWithChangeInfo } from '../NameWithChangeInfo/NameWithChangeInfo.tsx'; -import { useUiFlag } from 'hooks/useUiFlag.ts'; +import { NameWithChangeInfo } from '../Changes/Change/NameWithChangeInfo/NameWithChangeInfo.tsx'; const StyledCodeSection = styled('div')(({ theme }) => ({ overflowX: 'auto', @@ -41,9 +41,6 @@ const sortSegments = ( }; }; -const omitIfDefined = (obj: any, keys: string[]) => - obj ? omit(obj, keys) : obj; - export const StrategyDiff: FC<{ change: | IChangeRequestAddStrategy @@ -51,28 +48,21 @@ export const StrategyDiff: FC<{ | IChangeRequestDeleteStrategy; currentStrategy?: IFeatureStrategy; }> = ({ change, currentStrategy }) => { - const useNewDiff = useUiFlag('improvedJsonDiff'); const changeRequestStrategy = change.action === 'deleteStrategy' ? undefined : change.payload; const sortedCurrentStrategy = sortSegments(currentStrategy); const sortedChangeRequestStrategy = sortSegments(changeRequestStrategy); - const Wrapper = useNewDiff ? Fragment : StyledCodeSection; - const omissionFunction = useNewDiff ? omitIfDefined : omit; return ( - + - + ); };