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: <img width="999" alt="image" src="https://github.com/user-attachments/assets/26e2a987-f582-449d-b61c-bf2ec5c1edd4" /> Diff views: <img width="1011" alt="image" src="https://github.com/user-attachments/assets/95621234-1352-4164-8f74-775bdb0e61dd" />
2025-08-13 13:48:59 +02:00 · 2025-06-30 11:53:21 +02:00 · 2025-06-30 11:53:21 +02:00 · e2bb894f68
commit e2bb894f68
parent 2d2ba4ae25
8 changed files with 319 additions and 143 deletions
--- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeTabComponents.tsx
+++ 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) => (
+    <MuiTab slots={{ root: StyledButton }}>{children}</MuiTab>
+))(({ 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) => (
+    <MuiTabs
+        aria-label='View rendered change or JSON diff'
+        selectionFollowsFocus
+        defaultValue={0}
+    >
+        {children}
+    </MuiTabs>
+);
+
+export const TabPanel = MuiTabPanel;
--- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/LegacySegmentChangeDetails.tsx
+++ 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 (
+        <SegmentContainer conflict={change.conflict}>
+            {change.action === 'deleteSegment' && (
+                <ChangeItemWrapper>
+                    <ChangeItemInfo>
+                        <Typography
+                            sx={(theme) => ({
+                                color: theme.palette.error.main,
+                            })}
+                        >
+                            - Deleting segment:
+                        </Typography>
+                        <SegmentTooltipLink
+                            name={change.payload.name}
+                            previousName={previousName}
+                        >
+                            <SegmentDiff
+                                change={change}
+                                currentSegment={referenceSegment}
+                            />
+                        </SegmentTooltipLink>
+                    </ChangeItemInfo>
+                    <div>{actions}</div>
+                </ChangeItemWrapper>
+            )}
+            {change.action === 'updateSegment' && (
+                <>
+                    <ChangeOverwriteWarning
+                        data={{
+                            current: currentSegment,
+                            change,
+                            changeType: 'segment',
+                        }}
+                        changeRequestState={changeRequestState}
+                    />
+                    <ChangeItemCreateEditWrapper>
+                        <ChangeItemInfo>
+                            <Typography>Editing segment:</Typography>
+                            <SegmentTooltipLink name={change.payload.name}>
+                                <SegmentDiff
+                                    change={change}
+                                    currentSegment={referenceSegment}
+                                />
+                            </SegmentTooltipLink>
+                        </ChangeItemInfo>
+                        <div>{actions}</div>
+                    </ChangeItemCreateEditWrapper>
+                    <ViewableConstraintsList
+                        constraints={change.payload.constraints}
+                    />
+                </>
+            )}
+        </SegmentContainer>
+    );
+};
--- 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<ISegmentChangeProps> = ({
 }) => {
    const { segment } = useSegment(segmentChange.payload.id);

+    const ChangeDetails = useUiFlag('crDiffView')
+        ? SegmentChangeDetails
+        : LegacySegmentChangeDetails;
+
    return (
        <Card
            elevation={0}
@ -75,7 +81,7 @@ export const SegmentChange: FC<ISegmentChangeProps> = ({
                    </Link>
                </Box>
            </Box>
-            <SegmentChangeDetails
+            <ChangeDetails
                change={segmentChange}
                actions={actions}
                changeRequestState={changeRequestState}
--- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChangeDetails.tsx
+++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChangeDetails.tsx
@ -8,14 +8,13 @@ import type {
 } 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';

 const ChangeItemCreateEditWrapper = styled(Box)(({ theme }) => ({
-    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 = (
+        <>
+            <TabList>
+                <Tab>Change</Tab>
+                <Tab>View diff</Tab>
+            </TabList>
+            {actions}
+        </>
+    );
+
    return (
-        <SegmentContainer conflict={change.conflict}>
-            {change.action === 'deleteSegment' && (
-                <ChangeItemWrapper>
-                    <ChangeItemInfo>
-                        <Typography
-                            sx={(theme) => ({
-                                color: theme.palette.error.main,
-                            })}
-                        >
-                            - Deleting segment:
-                        </Typography>
-                        <SegmentTooltipLink
-                            name={change.payload.name}
-                            previousName={previousName}
-                        >
+        <Tabs>
+            <SegmentContainer conflict={change.conflict}>
+                {change.action === 'deleteSegment' && (
+                    <>
+                        <ChangeItemWrapper>
+                            <ChangeItemInfo>
+                                <Typography
+                                    sx={(theme) => ({
+                                        color: theme.palette.error.main,
+                                    })}
+                                >
+                                    - Deleting segment:
+                                </Typography>
+                                <SegmentTooltipLink
+                                    name={change.payload.name}
+                                    previousName={previousName}
+                                >
+                                    <SegmentDiff
+                                        change={change}
+                                        currentSegment={referenceSegment}
+                                    />
+                                </SegmentTooltipLink>
+                            </ChangeItemInfo>
+                            {actionsWithTabs}
+                        </ChangeItemWrapper>
+
+                        <TabPanel />
+                        <TabPanel>
                            <SegmentDiff
                                change={change}
                                currentSegment={referenceSegment}
                            />
-                        </SegmentTooltipLink>
-                    </ChangeItemInfo>
-                    <div>{actions}</div>
-                </ChangeItemWrapper>
-            )}
-            {change.action === 'updateSegment' && (
-                <>
-                    <ChangeOverwriteWarning
-                        data={{
-                            current: currentSegment,
-                            change,
-                            changeType: 'segment',
-                        }}
-                        changeRequestState={changeRequestState}
-                    />
-                    <ChangeItemCreateEditWrapper>
-                        <ChangeItemInfo>
-                            <Typography>Editing segment:</Typography>
-                            <SegmentTooltipLink name={change.payload.name}>
-                                <SegmentDiff
-                                    change={change}
-                                    currentSegment={referenceSegment}
-                                />
-                            </SegmentTooltipLink>
-                        </ChangeItemInfo>
-                        <div>{actions}</div>
-                    </ChangeItemCreateEditWrapper>
-                    <ViewableConstraintsList
-                        constraints={change.payload.constraints}
-                    />
-                </>
-            )}
-        </SegmentContainer>
+                        </TabPanel>
+                    </>
+                )}
+                {change.action === 'updateSegment' && (
+                    <>
+                        <ChangeOverwriteWarning
+                            data={{
+                                current: currentSegment,
+                                change,
+                                changeType: 'segment',
+                            }}
+                            changeRequestState={changeRequestState}
+                        />
+                        <ChangeItemCreateEditWrapper>
+                            <ChangeItemInfo>
+                                <Typography>Editing segment:</Typography>
+                                <SegmentTooltipLink name={change.payload.name}>
+                                    <SegmentDiff
+                                        change={change}
+                                        currentSegment={referenceSegment}
+                                    />
+                                </SegmentTooltipLink>
+                            </ChangeItemInfo>
+                            {actionsWithTabs}
+                        </ChangeItemCreateEditWrapper>
+
+                        <TabPanel>
+                            <ViewableConstraintsList
+                                constraints={change.payload.constraints}
+                            />
+                        </TabPanel>
+                        <TabPanel>
+                            <SegmentDiff
+                                change={change}
+                                currentSegment={referenceSegment}
+                            />
+                        </TabPanel>
+                    </>
+                )}
+            </SegmentContainer>
+        </Tabs>
    );
 };
--- 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) => (
-    <Tab slots={{ root: StyledButton }}>{children}</Tab>
-))(({ 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 = (
-        <ActionsContainer>
-            <StyledTabList>
-                <StyledTab>Change</StyledTab>
-                <StyledTab>View diff</StyledTab>
-            </StyledTabList>
+    const actionsWithTabs = (
+        <>
+            <TabList>
+                <Tab>Change</Tab>
+                <Tab>View diff</Tab>
+            </TabList>
            {actions}
-        </ActionsContainer>
+        </>
    );

    return (
-        <Tabs
-            aria-label='View rendered change or JSON diff'
-            selectionFollowsFocus
-            defaultValue={0}
-        >
+        <Tabs>
            {change.action === 'addStrategy' && (
-                <AddStrategy change={change} actions={Actions} />
+                <AddStrategy change={change} actions={actionsWithTabs} />
            )}
            {change.action === 'deleteStrategy' && (
                <DeleteStrategy
                    change={change}
                    changeRequestState={changeRequestState}
                    currentStrategy={currentStrategy}
-                    actions={Actions}
+                    actions={actionsWithTabs}
                />
            )}
            {change.action === 'updateStrategy' && (
@ -429,7 +379,7 @@ export const StrategyChange: FC<{
                    change={change}
                    changeRequestState={changeRequestState}
                    currentStrategy={currentStrategy}
-                    actions={Actions}
+                    actions={actionsWithTabs}
                />
            )}
        </Tabs>
--- 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 (
-        <StyledCodeSection>
+        <Wrapper>
            <EventDiff
                entry={{
-                    preData: omit(currentSegment, ['createdAt', 'createdBy']),
-                    data: omit(changeRequestSegment, ['snapshot']),
+                    preData: omissionFunction(currentSegment, [
+                        'createdAt',
+                        'createdBy',
+                    ]),
+                    data: omissionFunction(changeRequestSegment, ['snapshot']),
                }}
            />
-        </StyledCodeSection>
+        </Wrapper>
    );
 };
 interface IStrategyTooltipLinkProps {
--- 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 = <T extends { segments?: number[] }>(
    };
 };

+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 (
-            <NewEventDiff
-                entry={{
-                    preData: sortedCurrentStrategy
-                        ? omit(sortedCurrentStrategy, 'sortOrder')
-                        : undefined,
-                    data: sortedChangeRequestStrategy
-                        ? omit(sortedChangeRequestStrategy, 'snapshot')
-                        : undefined,
-                }}
-            />
-        );
-    }
+
+    const Wrapper = useNewDiff ? Fragment : StyledCodeSection;
+    const omissionFunction = useNewDiff ? omitIfDefined : omit;
    return (
-        <StyledCodeSection>
+        <Wrapper>
            <EventDiff
                entry={{
-                    preData: omit(sortedCurrentStrategy, 'sortOrder'),
-                    data: omit(sortedChangeRequestStrategy, 'snapshot'),
+                    preData: omissionFunction(sortedCurrentStrategy, [
+                        'sortOrder',
+                    ]),
+                    data: omissionFunction(sortedChangeRequestStrategy, [
+                        'snapshot',
+                    ]),
                }}
            />
-        </StyledCodeSection>
+        </Wrapper>
    );
 };

--- a/frontend/src/component/events/EventDiff/EventDiff.tsx
+++ b/frontend/src/component/events/EventDiff/EventDiff.tsx
@ -212,7 +212,7 @@ const OldEventDiff: FC<IEventDiffProps> = ({
    );
 };

-const EventDiff: FC<IEventDiffProps> = (props) => {
+export const EventDiff: FC<IEventDiffProps> = (props) => {
    const useNewJsonDiff = useUiFlag('improvedJsonDiff');
    if (useNewJsonDiff) {
        return <NewEventDiff {...props} />;