Skip to content

Commit

Permalink
Fix: weird strategy spacing on envs without release plans (#9466)
Browse files Browse the repository at this point in the history
Fixes a visual bug where envs without release plans would get too much
spacing on the top of their first strategy.

It does this flattening the list of strategies if there are no release
plans. In doing so, I have extracted the strategy list rendering into a
separate component (to make things more legible and re-usable) and have
also removed the FeatureStrategyEmpty component and marked it as
deprecated. In the new designs, you can't expand envs without
strategies, so the component is no longer needed.

Before (what looks like a shadow is actually the extra list being
rendered with a bit of padding):

![image](https://github.com/user-attachments/assets/5ba06ac9-046c-4fbd-8b46-b077b8a0570b)

After:

![image](https://github.com/user-attachments/assets/64270582-1221-4bdf-a85b-c24ce23bd4a3)
  • Loading branch information
thomasheartman authored Mar 10, 2025
1 parent f3dfb1d commit 51c9617
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 94 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// deprecated; remove with the `flagOverviewRedesign` flag
import { Link } from 'react-router-dom';
import { Box, styled } from '@mui/material';
import useFeatureStrategyApi from 'hooks/api/actions/useFeatureStrategyApi/useFeatureStrategyApi';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import useFeatureStrategyApi from 'hooks/api/actions/useFeatureStrategyApi/useFe
import { formatUnknownError } from 'utils/formatUnknownError';
import useToast from 'hooks/useToast';
import type { IFeatureEnvironment } from 'interfaces/featureToggle';
import { FeatureStrategyEmpty } from 'component/feature/FeatureStrategy/FeatureStrategyEmpty/FeatureStrategyEmpty';
import { useRequiredPathParam } from 'hooks/useRequiredPathParam';
import { useFeature } from 'hooks/api/getters/useFeature/useFeature';
import { useChangeRequestApi } from 'hooks/api/actions/useChangeRequestApi/useChangeRequestApi';
Expand Down Expand Up @@ -239,103 +238,85 @@ export const EnvironmentAccordionBody = ({
);
};

const Strategies = () => {
return strategies.length < 50 || !manyStrategiesPagination ? (
<StyledContentList>
{strategies.map((strategy, index) => (
<StyledListItem key={strategy.id}>
{index > 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>
<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>
);
};

return (
<div>
<StyledAccordionBodyInnerContainer>
{releasePlans.length > 0 || strategies.length > 0 ? (
<StyledContentList>
{releasePlans.map((plan) => (
<StyledListItem type='release plan' key={plan.id}>
<ReleasePlan
plan={plan}
environmentIsDisabled={isDisabled}
/>
</StyledListItem>
))}
<li>
{releasePlans.length > 0 ? (
<StrategySeparator />
) : null}
{strategies.length < 50 ||
!manyStrategiesPagination ? (
<StyledContentList>
{strategies.map((strategy, index) => (
<StyledListItem key={strategy.id}>
{index > 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>
<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)
}
<StyledContentList>
{releasePlans.length > 0 ? (
<>
{releasePlans.map((plan) => (
<StyledListItem
type='release plan'
key={plan.id}
>
<ReleasePlan
plan={plan}
environmentIsDisabled={isDisabled}
/>
</PaginatedStrategyContainer>
)}
</li>
</StyledContentList>
) : (
<FeatureStrategyEmpty
projectId={projectId}
featureId={featureId}
environmentId={featureEnvironment.name}
/>
)}
</StyledListItem>
))}
{strategies.length > 0 ? (
<li>
<StrategySeparator />
<Strategies />
</li>
) : null}
</>
) : strategies.length > 0 ? (
<Strategies />
) : null}
</StyledContentList>
</StyledAccordionBodyInnerContainer>
</div>
);
Expand Down

0 comments on commit 51c9617

Please sign in to comment.