From 64a6af2858a89cf3c6001417a365c8373f7be4b7 Mon Sep 17 00:00:00 2001
From: Thomas Heartman <thomas@getunleash.io>
Date: Fri, 16 Feb 2024 12:13:40 +0800
Subject: [PATCH] feat: show info on what would be deleted (#6235)

This PR updates the way we show deleted strategies in the CR UI. Instead
of showing just the strategy name and a diff on hover, we show the same
strategy config as we do for new and updated strategies.

This makes it easier to see what you have deleted.

In doing so, it also fixes two issues:
1. inconsistent border radius for segment changes listed. Due to an
override in `frontend/src/themes/theme.ts`, these would get a border
radius of `theme.shape.borderRadiusLarge` instead of
`theme.shape.borderRadiusMedium`. It does this by adding a class and
making the selector more specific.
2. The background was unset for the strategy rollout box and constraint
item boxes.

It looks like this:

<img width="728" alt="image"
src="https://github.com/Unleash/unleash/assets/17786332/7cba28ac-0454-444d-8cfa-f46543ccf2dc">

<img width="728" alt="image"
src="https://github.com/Unleash/unleash/assets/17786332/832be653-3def-4afc-b72f-36fcd76ad83d">

Or with more kinds of strategies:
<img width="454" alt="image"
src="https://github.com/Unleash/unleash/assets/17786332/f18e5482-7d2e-4cbd-8177-9de6dfb10307">


Note: I'm happy to isolate the color changes to a separate PR if that's
preferable.
---
 .../Changes/Change/FeatureChange.tsx          | 11 +++-
 .../Changes/Change/StrategyChange.tsx         | 64 +++++++++++--------
 .../ConstraintAccordionList.tsx               | 13 +++-
 .../common/SegmentItem/SegmentItem.tsx        |  6 +-
 .../ConstraintItem/ConstraintItem.tsx         |  1 +
 .../StrategyExecution/StrategyExecution.tsx   |  1 +
 6 files changed, 64 insertions(+), 32 deletions(-)

diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.tsx
index d111f3218a..5aae905dc7 100644
--- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.tsx
+++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/FeatureChange.tsx
@@ -66,6 +66,13 @@ const InlineList = styled('ul')(({ theme }) => ({
     },
 }));
 
+const ChangeInnerBox = styled(Box)(({ theme }) => ({
+    padding: theme.spacing(3),
+    '&:has(.delete-strategy-information-wrapper)': {
+        backgroundColor: theme.palette.error.light,
+    },
+}));
+
 export const FeatureChange: FC<{
     actions: ReactNode;
     index: number;
@@ -134,7 +141,7 @@ export const FeatureChange: FC<{
                 }
             />
 
-            <Box sx={(theme) => ({ padding: theme.spacing(3) })}>
+            <ChangeInnerBox>
                 {(change.action === 'addDependency' ||
                     change.action === 'deleteDependency') && (
                     <DependencyChange
@@ -183,7 +190,7 @@ export const FeatureChange: FC<{
                         actions={actions}
                     />
                 )}
-            </Box>
+            </ChangeInnerBox>
         </StyledSingleChangeBox>
     );
 };
diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx
index 5fbfb1ffcc..34963b07e8 100644
--- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx
+++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx
@@ -25,7 +25,7 @@ export const ChangeItemWrapper = styled(Box)({
     alignItems: 'center',
 });
 
-const ChangeItemCreateEditWrapper = styled(Box)(({ theme }) => ({
+const ChangeItemCreateEditDeleteWrapper = styled(Box)(({ theme }) => ({
     display: 'grid',
     gridTemplateColumns: 'auto auto',
     justifyContent: 'space-between',
@@ -142,7 +142,7 @@ export const StrategyChange: VFC<{
         <>
             {change.action === 'addStrategy' && (
                 <>
-                    <ChangeItemCreateEditWrapper>
+                    <ChangeItemCreateEditDeleteWrapper>
                         <ChangeItemInfo>
                             <Typography
                                 color={
@@ -167,7 +167,7 @@ export const StrategyChange: VFC<{
                             </div>
                         </ChangeItemInfo>
                         <div>{actions}</div>
-                    </ChangeItemCreateEditWrapper>
+                    </ChangeItemCreateEditDeleteWrapper>
                     <StrategyExecution strategy={change.payload} />
                     <ConditionallyRender
                         condition={hasVariantDiff}
@@ -187,26 +187,40 @@ export const StrategyChange: VFC<{
                 </>
             )}
             {change.action === 'deleteStrategy' && (
-                <ChangeItemWrapper>
-                    <ChangeItemInfo>
-                        <Typography
-                            sx={(theme) => ({
-                                color: theme.palette.error.main,
-                            })}
-                        >
-                            - Deleting strategy:
-                        </Typography>
-                        {hasNameField(change.payload) && (
-                            <StrategyTooltipLink change={change}>
-                                <StrategyDiff
-                                    change={change}
-                                    currentStrategy={currentStrategy}
-                                />
-                            </StrategyTooltipLink>
-                        )}
-                    </ChangeItemInfo>
-                    <div>{actions}</div>
-                </ChangeItemWrapper>
+                <>
+                    <ChangeItemCreateEditDeleteWrapper className='delete-strategy-information-wrapper'>
+                        <ChangeItemInfo>
+                            <Typography
+                                sx={(theme) => ({
+                                    color: theme.palette.error.main,
+                                })}
+                            >
+                                - Deleting strategy:
+                            </Typography>
+                            {hasNameField(change.payload) && (
+                                <StrategyTooltipLink change={change}>
+                                    <StrategyDiff
+                                        change={change}
+                                        currentStrategy={currentStrategy}
+                                    />
+                                </StrategyTooltipLink>
+                            )}
+                        </ChangeItemInfo>
+                        <div>{actions}</div>
+                    </ChangeItemCreateEditDeleteWrapper>
+                    <ConditionallyRender
+                        condition={Boolean(currentStrategy)}
+                        show={
+                            <Typography>
+                                {
+                                    <StrategyExecution
+                                        strategy={currentStrategy!}
+                                    />
+                                }
+                            </Typography>
+                        }
+                    />
+                </>
             )}
             {change.action === 'updateStrategy' && (
                 <>
@@ -214,7 +228,7 @@ export const StrategyChange: VFC<{
                         currentStrategy={currentStrategy}
                         change={change}
                     />
-                    <ChangeItemCreateEditWrapper>
+                    <ChangeItemCreateEditDeleteWrapper>
                         <ChangeItemInfo>
                             <EditHeader
                                 wasDisabled={currentStrategy?.disabled}
@@ -231,7 +245,7 @@ export const StrategyChange: VFC<{
                             </StrategyTooltipLink>
                         </ChangeItemInfo>
                         <div>{actions}</div>
-                    </ChangeItemCreateEditWrapper>
+                    </ChangeItemCreateEditDeleteWrapper>
                     <ConditionallyRender
                         condition={
                             change.payload?.disabled !==
diff --git a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionList/ConstraintAccordionList.tsx b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionList/ConstraintAccordionList.tsx
index bb39b3e5d5..24e00e21ef 100644
--- a/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionList/ConstraintAccordionList.tsx
+++ b/frontend/src/component/common/ConstraintAccordion/ConstraintAccordionList/ConstraintAccordionList.tsx
@@ -39,11 +39,15 @@ interface IConstraintAccordionListItemState {
 
 export const constraintAccordionListId = 'constraintAccordionListId';
 
-const StyledContainer = styled('div')({
+const StyledContainer = styled('div')(({ theme }) => ({
     width: '100%',
     display: 'flex',
     flexDirection: 'column',
-});
+    '&.constraint-list-element': {
+        borderRadius: theme.shape.borderRadiusMedium,
+        backgroundColor: theme.palette.background.default,
+    },
+}));
 
 const StyledHelpWrapper = styled(Tooltip)(({ theme }) => ({
     marginLeft: theme.spacing(0.75),
@@ -224,7 +228,10 @@ export const ConstraintList = forwardRef<
     }
 
     return (
-        <StyledContainer id={constraintAccordionListId}>
+        <StyledContainer
+            id={constraintAccordionListId}
+            className='constraint-list-element'
+        >
             {constraints.map((constraint, index) => (
                 <Fragment key={objectId(constraint)}>
                     <ConditionallyRender
diff --git a/frontend/src/component/common/SegmentItem/SegmentItem.tsx b/frontend/src/component/common/SegmentItem/SegmentItem.tsx
index 8db242b286..692ffc65b0 100644
--- a/frontend/src/component/common/SegmentItem/SegmentItem.tsx
+++ b/frontend/src/component/common/SegmentItem/SegmentItem.tsx
@@ -25,7 +25,9 @@ const StyledAccordion = styled(Accordion, {
     shouldForwardProp: (prop) => prop !== 'isDisabled',
 })<{ isDisabled: boolean | null }>(({ theme, isDisabled }) => ({
     border: `1px solid ${theme.palette.divider}`,
-    borderRadius: theme.shape.borderRadiusMedium,
+    '&.segment-accordion': {
+        borderRadius: theme.shape.borderRadiusMedium,
+    },
     boxShadow: 'none',
     margin: 0,
     transition: 'all 0.1s ease',
@@ -70,7 +72,7 @@ export const SegmentItem: VFC<ISegmentItemProps> = ({
     const [isOpen, setIsOpen] = useState(isExpanded || false);
 
     return (
-        <StyledAccordion isDisabled={disabled}>
+        <StyledAccordion className='segment-accordion' isDisabled={disabled}>
             <StyledAccordionSummary id={`segment-accordion-${segment.id}`}>
                 <DonutLarge
                     sx={(theme) => ({
diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/ConstraintItem/ConstraintItem.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/ConstraintItem/ConstraintItem.tsx
index 642499e9c2..91680bc098 100644
--- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/ConstraintItem/ConstraintItem.tsx
+++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/ConstraintItem/ConstraintItem.tsx
@@ -11,6 +11,7 @@ const StyledContainer = styled('div')(({ theme }) => ({
     width: '100%',
     padding: theme.spacing(2, 3),
     borderRadius: theme.shape.borderRadiusMedium,
+    background: theme.palette.background.default,
     border: `1px solid ${theme.palette.divider}`,
 }));
 
diff --git a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.tsx b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.tsx
index 80571b75a5..e28bbd0e67 100644
--- a/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.tsx
+++ b/frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.tsx
@@ -51,6 +51,7 @@ const StyledValueContainer = styled(Box)(({ theme }) => ({
     padding: theme.spacing(2, 3),
     border: `1px solid ${theme.palette.divider}`,
     borderRadius: theme.shape.borderRadiusMedium,
+    background: theme.palette.background.default,
 }));
 
 const StyledValueSeparator = styled('span')(({ theme }) => ({