1
0
mirror of https://github.com/Unleash/unleash.git synced 2025-04-15 01:16:22 +02:00

refactor: flatten release plan + strategy list (#9581)

Flattens the list of strategies when you have both release plans and
strategies. If you had both, you'd have this setup before:
```
- ol
  - li // release plan
    - ol // release plan strategies
  - li // regular strategies
    - ol // strategy list
```

Now we drop the extra nesting:
```
- ol
  -  li // release plan
    - ol // release plan strategies
  - li // the rest of the strategies
```

Semantically, I think this is just as valid and it simplifies a lot of
styling that no longer needs to look for other lists etc.

As part of doing this, I have also moved the "many strategies" warnings
and pagination labels to outside the list instead of inside the smaller
list.

Otherwise, the list looks just the same as before and drag-n-drop works
just fine.

(side note: these strategies shouldn't have drag handles 🤔 )

![image](https://github.com/user-attachments/assets/f27f451c-1b73-4f18-903f-153e379b54c1)


As a bonus, this PR also:
- Uses the disabled style separator for disabled strats in playground
and deletes some unused components I found.

Playground disabled strats (we probably don't want double orange badges;
I'll talk to UX):

![image](https://github.com/user-attachments/assets/a722b18f-f3d5-4d53-b093-b44912284748)
This commit is contained in:
Thomas Heartman 2025-03-20 11:16:44 +01:00 committed by GitHub
parent 038c10f612
commit afd24aa58a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 91 additions and 118 deletions

View File

@ -4,7 +4,7 @@ import {
useEffect,
useState,
} from 'react';
import { Alert, Pagination, styled } from '@mui/material';
import { Alert, Pagination, type Theme, styled } from '@mui/material';
import useFeatureStrategyApi from 'hooks/api/actions/useFeatureStrategyApi/useFeatureStrategyApi';
import { formatUnknownError } from 'utils/formatUnknownError';
import useToast from 'hooks/useToast';
@ -46,44 +46,41 @@ export const StyledContentList = styled('ol')(({ theme }) => ({
'& > li + li': {
borderTop: `1px solid ${theme.palette.divider}`,
},
'&:not(li > &) > li:first-of-type': {
// select first list elements in lists that are not directly nested
// within other lists.
'& > li:first-of-type': {
paddingTop: theme.spacing(1),
},
'& > li:has(> ol)': {
// nested lists add their own padding to their list items, so we don't want to double it up.
paddingBlock: 0,
}));
const releasePlanBackground = (theme: Theme) =>
theme.palette.background.elevation2;
const strategyBackground = (theme: Theme) =>
theme.palette.background.elevation1;
export const StyledListItem = styled('li')<{
'data-type'?: 'release-plan' | 'strategy';
}>(({ theme, ...props }) => ({
background:
props['data-type'] === 'release-plan'
? releasePlanBackground(theme)
: strategyBackground(theme),
}));
const AlertContainer = styled('div')(({ theme }) => ({
padding: theme.spacing(2),
paddingBottom: theme.spacing(0),
backgroundColor: strategyBackground(theme),
':has(+ ol>li[data-type="release-plan"])': {
backgroundColor: releasePlanBackground(theme),
},
}));
export const StyledListItem = styled('li', {
shouldForwardProp: (prop) => prop !== 'type',
})<{ type?: 'release plan' | 'strategy' }>(({ theme, type }) => ({
background:
type === 'release plan'
? theme.palette.background.elevation2
: theme.palette.background.elevation1,
}));
const PaginatedStrategyContainer = styled('div')(({ theme }) => ({
paddingTop: theme.spacing(2.5),
position: 'relative',
display: 'flex',
flexFlow: 'column nowrap',
gap: theme.spacing(2),
}));
const StyledAlert = styled(Alert)(({ theme }) => ({
marginInline: theme.spacing(2), // should consider finding a variable here
}));
const StyledStrategySeparator = styled(StrategySeparator)(({ theme }) => ({
[`&:has(+ *:not(ol) .${disabledStrategyClassName}), &:has(+ ol > li:first-of-type .${disabledStrategyClassName})`]:
// todo: consider exporting this into a shared thing or move it into the separator itself (either as a disabled prop or using the css here)
export const StyledStrategySeparator = styled(StrategySeparator)({
[`&:has(+ * .${disabledStrategyClassName}, + .${disabledStrategyClassName})`]:
{
filter: 'grayscale(1)',
},
}));
});
export const EnvironmentAccordionBody = ({
featureEnvironment,
@ -246,80 +243,77 @@ export const EnvironmentAccordionBody = ({
);
};
const strategyList =
strategies.length < 50 || !manyStrategiesPagination ? (
<StyledContentList>
{strategies.map((strategy, index) => (
<StyledListItem key={strategy.id}>
{index > 0 ? <StyledStrategySeparator /> : null}
const paginateStrategies =
strategies.length >= 50 && manyStrategiesPagination;
<ProjectEnvironmentStrategyDraggableItem
strategy={strategy}
index={index}
environmentName={featureEnvironment.name}
otherEnvironments={otherEnvironments}
isDragging={dragItem?.id === strategy.id}
onDragStartRef={onDragStartRef}
onDragOver={onDragOver(strategy.id)}
onDragEnd={onDragEnd}
return (
<StyledAccordionBodyInnerContainer>
{paginateStrategies ? (
<AlertContainer>
<Alert severity='warning'>
We noticed you're using a high number of activation
strategies. To ensure a more targeted approach, consider
leveraging constraints or segments.
</Alert>
</AlertContainer>
) : null}
<StyledContentList>
{releasePlans.map((plan) => (
<StyledListItem data-type='release-plan' key={plan.id}>
<ReleasePlan
plan={plan}
environmentIsDisabled={isDisabled}
/>
</StyledListItem>
))}
</StyledContentList>
) : (
<PaginatedStrategyContainer>
<StyledAlert severity='warning'>
We noticed you're using a high number of activation
strategies. To ensure a more targeted approach, consider
leveraging constraints or segments.
</StyledAlert>
<StyledContentList>
{page.map((strategy, index) => (
<StyledListItem key={strategy.id}>
{index > 0 ? <StyledStrategySeparator /> : null}
{paginateStrategies ? (
<>
{page.map((strategy, index) => (
<StyledListItem key={strategy.id}>
{index > 0 || releasePlans.length > 0 ? (
<StyledStrategySeparator />
) : null}
<ProjectEnvironmentStrategyDraggableItem
strategy={strategy}
index={index + pageIndex * pageSize}
environmentName={featureEnvironment.name}
otherEnvironments={otherEnvironments}
/>
</StyledListItem>
))}
</StyledContentList>
<ProjectEnvironmentStrategyDraggableItem
strategy={strategy}
index={index + pageIndex * pageSize}
environmentName={featureEnvironment.name}
otherEnvironments={otherEnvironments}
/>
</StyledListItem>
))}
</>
) : (
<>
{strategies.map((strategy, index) => (
<StyledListItem key={strategy.id}>
{index > 0 || releasePlans.length > 0 ? (
<StyledStrategySeparator />
) : null}
<ProjectEnvironmentStrategyDraggableItem
strategy={strategy}
index={index}
environmentName={featureEnvironment.name}
otherEnvironments={otherEnvironments}
isDragging={dragItem?.id === strategy.id}
onDragStartRef={onDragStartRef}
onDragOver={onDragOver(strategy.id)}
onDragEnd={onDragEnd}
/>
</StyledListItem>
))}
</>
)}
</StyledContentList>
{paginateStrategies ? (
<Pagination
count={pages.length}
shape='rounded'
page={pageIndex + 1}
onChange={(_, page) => setPageIndex(page - 1)}
/>
</PaginatedStrategyContainer>
);
return (
<StyledAccordionBodyInnerContainer>
<StyledContentList>
{releasePlans.length > 0 ? (
<>
{releasePlans.map((plan) => (
<StyledListItem type='release plan' key={plan.id}>
<ReleasePlan
plan={plan}
environmentIsDisabled={isDisabled}
/>
</StyledListItem>
))}
{strategies.length > 0 ? (
<li>
<StyledStrategySeparator />
{strategyList}
</li>
) : null}
</>
) : strategies.length > 0 ? (
strategyList
) : null}
</StyledContentList>
) : null}
</StyledAccordionBodyInnerContainer>
);
};

View File

@ -1,4 +1,4 @@
import { Alert, styled } from '@mui/material';
import { styled } from '@mui/material';
import type {
PlaygroundStrategySchema,
PlaygroundRequestSchema,
@ -6,29 +6,10 @@ import type {
import {
StyledContentList,
StyledListItem,
StyledStrategySeparator,
} from 'component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/EnvironmentAccordionBody';
import { StrategySeparator } from 'component/common/StrategySeparator/StrategySeparator';
import { FeatureStrategyItem } from './StrategyItem/FeatureStrategyItem';
const StyledAlertWrapper = styled('div')(({ theme }) => ({
display: 'flex',
padding: `0, 4px`,
flexDirection: 'column',
borderRadius: theme.shape.borderRadiusMedium,
border: `1px solid ${theme.palette.warning.border}`,
}));
const StyledListWrapper = styled('div')(({ theme }) => ({
padding: theme.spacing(1, 0.5),
}));
const StyledAlert = styled(Alert)(({ theme }) => ({
border: '0!important',
borderBottomLeftRadius: 0,
borderBottomRightRadius: 0,
borderBottom: `1px solid ${theme.palette.warning.border}!important`,
}));
interface PlaygroundResultStrategyListProps {
strategies: PlaygroundStrategySchema[];
input?: PlaygroundRequestSchema;
@ -85,7 +66,7 @@ export const PlaygroundResultStrategyLists = ({
<RestyledContentList>
{strategies?.map((strategy, index) => (
<StyledListItem key={strategy.id}>
{index > 0 ? <StrategySeparator /> : ''}
{index > 0 ? <StyledStrategySeparator /> : ''}
<FeatureStrategyItem
strategy={strategy}
input={input}

View File

@ -1,4 +1,3 @@
import { useTheme } from '@mui/material';
import { PlaygroundResultChip } from '../../../../PlaygroundResultChip/PlaygroundResultChip';
import type {
PlaygroundStrategySchema,
@ -20,7 +19,6 @@ export const FeatureStrategyItem = ({
className,
}: IFeatureStrategyItemProps) => {
const { result } = strategy;
const theme = useTheme();
const label =
result.evaluationStatus === 'incomplete' ||
result.evaluationStatus === 'unevaluated'