Mirrors/unleash.unleash
1
0
Fork 0
mirror of https://github.com/Unleash/unleash.git synced 2025-08-23 13:46:45 +02:00
Code Issues Projects Releases Wiki Activity

chore(1-3450): Place strategy names and titles on the same line (and fix list nesting issues) (#9443)

Browse Source 
Moves strategy titles and names onto the same line, as per the new
designs.

In doing so, I've also updated the component to use a more semantic
hgroup with the header being the strategy title if it exists or the
strategy name if not.

The downside of being more semantically correct here is that we need to
know what header level we want the strategy to use. In most cases,
that's 3 (e.g. flag name > environment > strategy, release plan >
milestone > strategy), but for plans on flag envs, it's 4 (flag name >
env > milestone name > strategy).

I've also taken the opportunity to fix a little mistake I made earlier.
`ol`s can only have `li` children, and I'd forgotten to wrap a nested
`ol` inside an `li`. The changes in `EnvironmentAccordionBody` all
relate to that change. Because we now have several layers of lists
nested within each other, dealing with styling and padding gets a little
tricky, but CSS has the power do help us out here.

Rendered:

![image](https://github.com/user-attachments/assets/634615fe-b06d-4baa-8aa3-22447d1fb8b4)
This commit is contained in:
Thomas Heartman 2025-03-07 13:09:36 +01:00 committed by GitHub
parent f6987909e7
commit 220550071f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 126 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
frontend/src/component/common/StrategyItemContainer/StrategyItemContainer.test.tsx
View File

@ -42,7 +42,7 @@ test('should render strategy name, custom title and description', async () => {
/>,
);
expect(screen.getByText('strategy name')).toBeInTheDocument();
expect(screen.getByText('strategy name:')).toBeInTheDocument();
expect(screen.getByText('description')).toBeInTheDocument();
await screen.findByText('custom title'); // behind async flag
});

83
frontend/src/component/common/StrategyItemContainer/StrategyItemContainer.tsx
View File

@ -1,16 +1,16 @@
import type React from 'react';
import type { DragEventHandler, FC, ReactNode } from 'react';
import DragIndicator from '@mui/icons-material/DragIndicator';
import { Box, IconButton, styled } from '@mui/material';
import { Box, IconButton, Typography, styled } from '@mui/material';
import type { IFeatureStrategy } from 'interfaces/strategy';
import { formatStrategyName } from 'utils/strategyNames';
import StringTruncator from 'component/common/StringTruncator/StringTruncator';
import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
import type { PlaygroundStrategySchema } from 'openapi';
import { Badge } from '../Badge/Badge';
import { Link } from 'react-router-dom';
type StrategyItemContainerProps = {
strategyHeaderLevel?: 1 | 2 | 3 | 4 | 5 | 6;
strategy: IFeatureStrategy | PlaygroundStrategySchema;
onDragStart?: DragEventHandler<HTMLButtonElement>;
onDragEnd?: DragEventHandler<HTMLButtonElement>;
@ -44,15 +44,17 @@ const StyledCustomTitle = styled('div')(({ theme }) => ({
display: 'block',
},
}));
const StyledHeaderContainer = styled('div')({
flexDirection: 'column',
justifyContent: 'center',
verticalAlign: 'middle',
});
const StyledHeaderContainer = styled('hgroup')(({ theme }) => ({
display: 'flex',
flexFlow: 'row',
columnGap: '1ch',
fontSize: theme.typography.body1.fontSize,
'.strategy-name': {
fontWeight: 'bold',
},
}));
const NewStyledContainer = styled(Box, {
shouldForwardProp: (prop) => prop !== 'disabled',
})({
const StyledContainer = styled('article')({
background: 'inherit',
});
@ -64,7 +66,6 @@ const NewStyledHeader = styled('div', {
display: 'flex',
gap: theme.spacing(1),
alignItems: 'center',
fontWeight: theme.typography.fontWeightMedium,
paddingLeft: draggable ? theme.spacing(1) : theme.spacing(2),
color: disabled
? theme.palette.text.secondary
@ -77,18 +78,19 @@ export const StrategyItemContainer: FC<StrategyItemContainerProps> = ({
onDragStart,
onDragEnd,
headerItemsRight,
strategyHeaderLevel = 3,
children,
style = {},
description,
}) => {
const StrategyHeaderLink: React.FC<{ children?: React.ReactNode }> =
'links' in strategy
'links' in strategy // todo: revisit this when we get to playground, related to flag `flagOverviewRedesign`
? ({ children }) => <Link to={strategy.links.edit}>{children}</Link>
: ({ children }) => <> {children} </>;
return (
<Box sx={{ position: 'relative' }}>
<NewStyledContainer style={style}>
<StyledContainer style={style}>
<NewStyledHeader
draggable={Boolean(onDragStart)}
disabled={Boolean(strategy?.disabled)}
@ -112,33 +114,40 @@ export const StrategyItemContainer: FC<StrategyItemContainerProps> = ({
</DragIcon>
)}
/>
<StyledHeaderContainer>
<StrategyHeaderLink>
<StringTruncator
maxWidth='400'
maxLength={15}
text={formatStrategyName(String(strategy.name))}
/>
<ConditionallyRender
condition={Boolean(strategy.title)}
show={
<StyledCustomTitle>
<StrategyHeaderLink>
<StyledHeaderContainer>
{strategy.title ? (
<>
<p className='strategy-name'>
{formatStrategyName(
String(strategy.title),
String(strategy.name),
)}
</StyledCustomTitle>
:
</p>
<Typography
component={`h${strategyHeaderLevel}`}
>
{strategy.title}
</Typography>
</>
) : (
<Typography
className='strategy-name'
component={`h${strategyHeaderLevel}`}
>
{formatStrategyName(String(strategy.name))}
</Typography>
)}
<ConditionallyRender
condition={Boolean(description)}
show={
<StyledDescription>
{description}
</StyledDescription>
}
/>
</StrategyHeaderLink>
<ConditionallyRender
condition={Boolean(description)}
show={
<StyledDescription>
{description}
</StyledDescription>
}
/>
</StyledHeaderContainer>
</StyledHeaderContainer>
</StrategyHeaderLink>
<ConditionallyRender
condition={Boolean(strategy?.disabled)}
@ -160,7 +169,7 @@ export const StrategyItemContainer: FC<StrategyItemContainerProps> = ({
</Box>
</NewStyledHeader>
<Box sx={{ p: 2, pt: 0 }}>{children}</Box>
</NewStyledContainer>
</StyledContainer>
</Box>
);
};

134
frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/EnvironmentAccordionBody.tsx
View File

@ -36,11 +36,25 @@ const StyledAccordionBodyInnerContainer = styled('div')(({ theme }) => ({
},
}));
export const StyledContentList = styled('ol')({
export const StyledContentList = styled('ol')(({ theme }) => ({
listStyle: 'none',
padding: 0,
margin: 0,
});
'& > li': {
paddingBlock: theme.spacing(2.5),
position: 'relative',
},
'&:not(li > &) > li:first-of-type': {
// select first list elements in lists that are not directly nested
// within other lists.
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,
},
}));
export const StyledListItem = styled('li', {
shouldForwardProp: (prop) => prop !== 'type',
@ -50,11 +64,6 @@ export const StyledListItem = styled('li', {
type === 'release plan'
? theme.palette.background.elevation2
: theme.palette.background.elevation1,
position: 'relative',
paddingBlock: theme.spacing(2.5),
'&:first-of-type': {
paddingTop: theme.spacing(1),
},
}));
const PaginatedStrategyContainer = styled('div')(({ theme }) => ({
@ -243,75 +252,82 @@ export const EnvironmentAccordionBody = ({
/>
</StyledListItem>
))}
{strategies.length < 50 || !manyStrategiesPagination ? (
<StyledContentList>
{strategies.map((strategy, index) => (
<StyledListItem key={strategy.id}>
{index > 0 ||
releasePlans.length > 0 ? (
<StrategySeparator />
) : 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>
) : (
<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>
<li>
{releasePlans.length > 0 ? (
<StrategySeparator />
) : null}
{strategies.length < 50 ||
!manyStrategiesPagination ? (
<StyledContentList>
{page.map((strategy, index) => (
{strategies.map((strategy, index) => (
<StyledListItem key={strategy.id}>
{index > 0 ||
releasePlans.length > 0 ? (
{index > 0 ? (
<StrategySeparator />
) : null}
<ProjectEnvironmentStrategyDraggableItem
strategy={strategy}
index={
index + pageIndex * pageSize
}
index={index}
environmentName={
featureEnvironment.name
}
otherEnvironments={
otherEnvironments
}
isDragging={
dragItem?.id === strategy.id
}
onDragStartRef={onDragStartRef}
onDragOver={onDragOver(
strategy.id,
)}
onDragEnd={onDragEnd}
/>
</StyledListItem>
))}
</StyledContentList>
<Pagination
count={pages.length}
shape='rounded'
page={pageIndex + 1}
onChange={(_, page) =>
setPageIndex(page - 1)
}
/>
</PaginatedStrategyContainer>
)}
) : (
<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 ? (
<StrategySeparator />
) : null}
<ProjectEnvironmentStrategyDraggableItem
strategy={strategy}
index={
index +
pageIndex * pageSize
}
environmentName={
featureEnvironment.name
}
otherEnvironments={
otherEnvironments
}
/>
</StyledListItem>
))}
</StyledContentList>
<Pagination
count={pages.length}
shape='rounded'
page={pageIndex + 1}
onChange={(_, page) =>
setPageIndex(page - 1)
}
/>
</PaginatedStrategyContainer>
)}
</li>
</StyledContentList>
) : (
<FeatureStrategyEmpty

3
frontend/src/component/feature/FeatureView/FeatureOverview/FeatureOverviewEnvironments/FeatureOverviewEnvironment/EnvironmentAccordionBody/StrategyDraggableItem/StrategyItem/StrategyItem.tsx
View File

@ -10,6 +10,7 @@ type StrategyItemProps = {
strategy: IFeatureStrategy;
onDragStart?: DragEventHandler<HTMLButtonElement>;
onDragEnd?: DragEventHandler<HTMLButtonElement>;
strategyHeaderLevel?: 1 | 2 | 3 | 4 | 5 | 6;
};
export const StrategyItem: FC<StrategyItemProps> = ({
@ -17,9 +18,11 @@ export const StrategyItem: FC<StrategyItemProps> = ({
onDragStart,
onDragEnd,
headerItemsRight,
strategyHeaderLevel,
}) => {
return (
<NewStrategyItemContainer
strategyHeaderLevel={strategyHeaderLevel}
strategy={strategy}
onDragStart={onDragStart}
onDragEnd={onDragEnd}

1
frontend/src/component/feature/FeatureView/FeatureOverview/ReleasePlan/ReleasePlanMilestone/ReleasePlanMilestone.tsx
View File

@ -123,6 +123,7 @@ export const ReleasePlanMilestone = ({
{index > 0 ? <StrategySeparator /> : null}
<StrategyItem
strategyHeaderLevel={4}
strategy={{
...strategy,
name: