Skip to content

Commit 038c10f

Browse files
thomasheartmanthomasheartman
authored
chore: finish up disabled strategies (#9579)
Aligns the design of disabled strategies with the sketches. Most notable changes: - makes the disabled badge warning yellow - greys out the preceding "or" separator - makes the segment "preview" button *not* grey (because it's still interactable) As a bonus: uses a list for the constraint value lists instead of a div and updates the design to match the sketches (no chips). ![image](https://github.com/user-attachments/assets/1b3ddfa0-b0e8-4856-ae01-26e507590a4f) With strat variants: ![image](https://github.com/user-attachments/assets/dc143fbf-256b-4e96-872b-a6aa84df2111) Bonus fix: Lets the constraint value list wrap so that we avoid this kind of blowout: ![image](https://github.com/user-attachments/assets/4c0977ac-f8a4-41cc-8fb7-194e8b09c0a3) Instead: ![image](https://github.com/user-attachments/assets/a68ed4cf-c68c-43a1-9d6c-b90e85a0841f)
1 parent f5b2634 commit 038c10f

File tree

7 files changed

+63
-38
lines changed

7 files changed

+63
-38
lines changed

‎frontend/src/component/common/ConstraintsList/StrategyEvaluationItem/StrategyEvaluationItem.tsx

+21-8
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { Chip, type ChipProps, styled } from '@mui/material';
1+
import { styled } from '@mui/material';
2+
import { disabledStrategyClassName } from 'component/common/StrategyItemContainer/disabled-strategy-utils';
23
import type { FC, ReactNode } from 'react';
34

45
type StrategyItemProps = {
@@ -19,6 +20,10 @@ const StyledContent = styled('div')(({ theme }) => ({
1920
gap: theme.spacing(1),
2021
flexWrap: 'wrap',
2122
alignItems: 'center',
23+
[`.${disabledStrategyClassName} &`]: {
24+
filter: 'grayscale(1)',
25+
color: theme.palette.text.secondary,
26+
},
2227
}));
2328

2429
const StyledType = styled('span')(({ theme }) => ({
@@ -30,17 +35,23 @@ const StyledType = styled('span')(({ theme }) => ({
3035
width: theme.spacing(10),
3136
}));
3237

33-
const StyledValuesGroup = styled('div')(({ theme }) => ({
38+
const StyledValuesGroup = styled('ul')(({ theme }) => ({
3439
display: 'flex',
40+
flexFlow: 'row wrap',
3541
alignItems: 'center',
3642
gap: theme.spacing(0.5),
43+
listStyle: 'none',
44+
padding: 0,
3745
}));
3846

39-
const StyledValue = styled(({ ...props }: ChipProps) => (
40-
<Chip size='small' {...props} />
41-
))(({ theme }) => ({
42-
padding: theme.spacing(0.5),
43-
background: theme.palette.background.elevation1,
47+
const StyledValue = styled('li')(({ theme }) => ({
48+
[`.${disabledStrategyClassName} &`]: {
49+
filter: 'grayscale(1)',
50+
color: theme.palette.text.secondary,
51+
},
52+
':not(&:last-of-type)::after': {
53+
content: '", "',
54+
},
4455
}));
4556

4657
/**
@@ -58,7 +69,9 @@ export const StrategyEvaluationItem: FC<StrategyItemProps> = ({
5869
{values && values?.length > 0 ? (
5970
<StyledValuesGroup>
6071
{values?.map((value, index) => (
61-
<StyledValue key={`${value}#${index}`} label={value} />
72+
<StyledValue key={`${value}#${index}`}>
73+
{value}
74+
</StyledValue>
6275
))}
6376
</StyledValuesGroup>
6477
) : null}

‎frontend/src/component/common/StrategyItemContainer/StrategyItemContainer.tsx

+8-4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { PlaygroundStrategySchema } from 'openapi';
88
import { Badge } from '../Badge/Badge';
99
import { Link } from 'react-router-dom';
1010
import { Truncator } from '../Truncator/Truncator';
11+
import { disabledStrategyClassName } from './disabled-strategy-utils';
1112

1213
type StrategyItemContainerProps = {
1314
strategyHeaderLevel?: 1 | 2 | 3 | 4 | 5 | 6;
@@ -87,7 +88,10 @@ export const StrategyItemContainer: FC<StrategyItemContainerProps> = ({
8788
: ({ children }) => <> {children} </>;
8889

8990
return (
90-
<Box sx={{ position: 'relative' }}>
91+
<Box
92+
className={strategy.disabled ? disabledStrategyClassName : ''}
93+
sx={{ position: 'relative' }}
94+
>
9195
<StyledContainer style={style} className={className}>
9296
<StyledHeader disabled={Boolean(strategy?.disabled)}>
9397
{onDragStart ? (
@@ -136,9 +140,6 @@ export const StrategyItemContainer: FC<StrategyItemContainerProps> = ({
136140
</StyledHeaderContainer>
137141
</StrategyHeaderLink>
138142

139-
{strategy.disabled ? (
140-
<Badge color='disabled'>Disabled</Badge>
141-
) : null}
142143
{headerItemsLeft}
143144
</StyledHeaderInner>
144145
<Box
@@ -148,6 +149,9 @@ export const StrategyItemContainer: FC<StrategyItemContainerProps> = ({
148149
alignItems: 'center',
149150
}}
150151
>
152+
{strategy.disabled ? (
153+
<Badge color='warning'>Strategy disabled</Badge>
154+
) : null}
151155
{headerItemsRight}
152156
</Box>
153157
</StyledHeader>

‎frontend/src/component/common/StrategyItemContainer/disabled-strategy-utils.ts

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export const disabledStrategyClassName =
2+
'disabled-strategy-55d37c31-ca3f-4c19-bf86-9158824899bf';

‎frontend/src/component/common/StrategySeparator/StrategySeparator.tsx

+6-2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ const Chip = styled('div')(({ theme }) => ({
1313
left: theme.spacing(4),
1414
}));
1515

16-
export const StrategySeparator = () => {
17-
return <Chip role='separator'>OR</Chip>;
16+
export const StrategySeparator = ({ className }: { className?: string }) => {
17+
return (
18+
<Chip role='separator' className={className}>
19+
OR
20+
</Chip>
21+
);
1822
};

‎frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/EnvironmentAccordionBody.tsx

+11-3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { useReleasePlans } from 'hooks/api/getters/useReleasePlans/useReleasePla
2222
import { ReleasePlan } from '../../../ReleasePlan/ReleasePlan';
2323
import { StrategySeparator } from 'component/common/StrategySeparator/StrategySeparator';
2424
import { ProjectEnvironmentStrategyDraggableItem } from './StrategyDraggableItem/ProjectEnvironmentStrategyDraggableItem';
25+
import { disabledStrategyClassName } from 'component/common/StrategyItemContainer/disabled-strategy-utils';
2526

2627
interface IEnvironmentAccordionBodyProps {
2728
isDisabled: boolean;
@@ -77,6 +78,13 @@ const StyledAlert = styled(Alert)(({ theme }) => ({
7778
marginInline: theme.spacing(2), // should consider finding a variable here
7879
}));
7980

81+
const StyledStrategySeparator = styled(StrategySeparator)(({ theme }) => ({
82+
[`&:has(+ *:not(ol) .${disabledStrategyClassName}), &:has(+ ol > li:first-of-type .${disabledStrategyClassName})`]:
83+
{
84+
filter: 'grayscale(1)',
85+
},
86+
}));
87+
8088
export const EnvironmentAccordionBody = ({
8189
featureEnvironment,
8290
isDisabled,
@@ -243,7 +251,7 @@ export const EnvironmentAccordionBody = ({
243251
<StyledContentList>
244252
{strategies.map((strategy, index) => (
245253
<StyledListItem key={strategy.id}>
246-
{index > 0 ? <StrategySeparator /> : null}
254+
{index > 0 ? <StyledStrategySeparator /> : null}
247255

248256
<ProjectEnvironmentStrategyDraggableItem
249257
strategy={strategy}
@@ -268,7 +276,7 @@ export const EnvironmentAccordionBody = ({
268276
<StyledContentList>
269277
{page.map((strategy, index) => (
270278
<StyledListItem key={strategy.id}>
271-
{index > 0 ? <StrategySeparator /> : null}
279+
{index > 0 ? <StyledStrategySeparator /> : null}
272280

273281
<ProjectEnvironmentStrategyDraggableItem
274282
strategy={strategy}
@@ -303,7 +311,7 @@ export const EnvironmentAccordionBody = ({
303311
))}
304312
{strategies.length > 0 ? (
305313
<li>
306-
<StrategySeparator />
314+
<StyledStrategySeparator />
307315
{strategyList}
308316
</li>
309317
) : null}

‎frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/ProjectEnvironmentStrategyDraggableItem.tsx

+3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { UPDATE_FEATURE_STRATEGY } from '@server/types/permissions';
1717
import { StrategyDraggableItem } from './StrategyDraggableItem';
1818

1919
type ProjectEnvironmentStrategyDraggableItemProps = {
20+
className?: string;
2021
strategy: IFeatureStrategy;
2122
environmentName: string;
2223
index: number;
@@ -36,6 +37,7 @@ type ProjectEnvironmentStrategyDraggableItemProps = {
3637
const onDragNoOp = () => () => {};
3738

3839
export const ProjectEnvironmentStrategyDraggableItem = ({
40+
className,
3941
strategy,
4042
index,
4143
environmentName,
@@ -74,6 +76,7 @@ export const ProjectEnvironmentStrategyDraggableItem = ({
7476

7577
return (
7678
<Box
79+
className={className}
7780
key={strategy.id}
7881
ref={ref}
7982
onDragOver={onDragOver(ref, index)}

‎frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyExecution/StrategyExecution.tsx

+12-21
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import type { FC } from 'react';
2-
import { styled } from '@mui/material';
32
import type { FeatureStrategySchema } from 'openapi';
43
import type { IFeatureStrategyPayload } from 'interfaces/strategy';
54
import { useUiFlag } from 'hooks/useUiFlag';
@@ -13,12 +12,6 @@ import { useSegments } from 'hooks/api/getters/useSegments/useSegments';
1312
import { SegmentItem } from 'component/common/SegmentItem/SegmentItem';
1413
import { ConstraintsList } from 'component/common/ConstraintsList/ConstraintsList';
1514

16-
const FilterContainer = styled('div', {
17-
shouldForwardProp: (prop) => prop !== 'grayscale',
18-
})<{ grayscale: boolean }>(({ grayscale }) =>
19-
grayscale ? { filter: 'grayscale(1)', opacity: 0.67 } : {},
20-
);
21-
2215
type StrategyExecutionProps = {
2316
strategy: IFeatureStrategyPayload | FeatureStrategySchema;
2417
displayGroupId?: boolean;
@@ -52,19 +45,17 @@ export const StrategyExecution: FC<StrategyExecutionProps> = ({
5245
}
5346

5447
return (
55-
<FilterContainer grayscale={strategy.disabled === true}>
56-
<ConstraintsList>
57-
{strategySegments?.map((segment) => (
58-
<SegmentItem segment={segment} key={segment.id} />
59-
))}
60-
{constraints?.map((constraint, index) => (
61-
<ConstraintItem
62-
key={`${objectId(constraint)}-${index}`}
63-
{...constraint}
64-
/>
65-
))}
66-
{isCustomStrategy ? customStrategyItems : strategyParameters}
67-
</ConstraintsList>
68-
</FilterContainer>
48+
<ConstraintsList>
49+
{strategySegments?.map((segment) => (
50+
<SegmentItem segment={segment} key={segment.id} />
51+
))}
52+
{constraints?.map((constraint, index) => (
53+
<ConstraintItem
54+
key={`${objectId(constraint)}-${index}`}
55+
{...constraint}
56+
/>
57+
))}
58+
{isCustomStrategy ? customStrategyItems : strategyParameters}
59+
</ConstraintsList>
6960
);
7061
};

0 commit comments

Comments
 (0)