Skip to content

flex: cleanup Flex* custom styling in favor of Flex primitive #93809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jun 20, 2025
Merged

flex: cleanup Flex* custom styling in favor of Flex primitive #93809

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f647847
flex: cleanup Flex* classes
JonasBa Jun 18, 2025
2345d0a
ref: revert flex change in tracedetailsheader
JonasBa Jun 18, 2025
fadc8c7
ref: use TextOverflow in group times
JonasBa Jun 18, 2025
3788afb
revert discover flex change
JonasBa Jun 18, 2025
a820348
revert groupEventAttachments flex change
JonasBa Jun 18, 2025
7f64ee0
alternative approach for AlertListRow
JonasBa Jun 18, 2025
11b9bf5
revert AlertListRow until we have min-width on flex
JonasBa Jun 18, 2025
5d00d6c
fix flex in waterfall
JonasBa Jun 18, 2025
fbf581b
fix flex in gsadmin
JonasBa Jun 18, 2025
5707e3e
remove ephemeral stories
JonasBa Jun 18, 2025
54c5f8d
revert samplerateinput flex change
JonasBa Jun 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions static/app/components/codeSnippet.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import styled from '@emotion/styled';
import Prism from 'prismjs';

import {Button} from 'sentry/components/core/button';
import {Flex} from 'sentry/components/core/layout';
import {IconCopy} from 'sentry/icons';
import {t} from 'sentry/locale';
import {space} from 'sentry/styles/space';
Expand Down Expand Up @@ -163,12 +164,12 @@ export function CodeSnippet({
</Tab>
))}
</TabsWrapper>
<FlexSpacer />
<Flex.Item grow={1} />
Copy link
Member

@natemoo-re natemoo-re Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if we should have <Flex.Spacer /> as a shorthand here? Seems easier to remember than this.

Copy link
Member Author

@JonasBa JonasBa Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that could also work. I don't have a strong opinion on it tbh, it seems like an edge case we can iron out later

</Fragment>
)}
{icon}
{filename && <FileName>{filename}</FileName>}
{!hasTabs && <FlexSpacer />}
{!hasTabs && <Flex.Item grow={1} />}
{!hideCopyButton && (
<CopyButton
type="button"
Expand Down Expand Up @@ -276,10 +277,6 @@ const Tab = styled('button')<{isSelected: boolean}>`
: ''}
`;

const FlexSpacer = styled('div')`
flex-grow: 1;
`;

const CopyButton = styled(Button)<{isAlwaysVisible: boolean}>`
color: var(--prism-comment);
transition: opacity 0.1s ease-out;
Expand Down
11 changes: 3 additions & 8 deletions static/app/components/codecov/branchSelector/branchSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import styled from '@emotion/styled';
import {useCodecovContext} from 'sentry/components/codecov/context/codecovContext';
import type {SelectOption} from 'sentry/components/core/compactSelect';
import {CompactSelect} from 'sentry/components/core/compactSelect';
import {Flex} from 'sentry/components/core/layout';
import DropdownButton from 'sentry/components/dropdownButton';
import {t} from 'sentry/locale';
import {space} from 'sentry/styles/space';
Expand Down Expand Up @@ -60,12 +61,12 @@ export function BranchSelector() {
{...triggerProps}
>
<TriggerLabelWrap>
<FlexContainer>
<Flex align="center" gap={space(0.75)}>
<IconContainer>
<IconBranch />
</IconContainer>
<TriggerLabel>{branch || t('Select branch')}</TriggerLabel>
</FlexContainer>
</Flex>
</TriggerLabelWrap>
</DropdownButton>
);
Expand Down Expand Up @@ -95,12 +96,6 @@ const OptionLabel = styled('span')`
}
`;

const FlexContainer = styled('div')`
display: flex;
align-items: center;
gap: ${space(0.75)};
`;

const IconContainer = styled('div')`
flex: 1 0 14px;
height: 14px;
Expand Down
11 changes: 3 additions & 8 deletions static/app/components/codecov/datePicker/dateSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import styled from '@emotion/styled';

import type {SelectOption, SingleSelectProps} from 'sentry/components/core/compactSelect';
import {CompactSelect} from 'sentry/components/core/compactSelect';
import {Flex} from 'sentry/components/core/layout';
import DropdownButton from 'sentry/components/dropdownButton';
import {getArbitraryRelativePeriod} from 'sentry/components/timeRangeSelector/utils';
import {IconCalendar} from 'sentry/icons/iconCalendar';
Expand Down Expand Up @@ -80,10 +81,10 @@ export function DateSelector({relativeDate, onChange, trigger}: DateSelectorProp
{...triggerProps}
>
<TriggerLabelWrap>
<FlexContainer>
<Flex align="center" gap={space(0.75)}>
<IconCalendar />
<TriggerLabel>{defaultLabel}</TriggerLabel>
</FlexContainer>
</Flex>
</TriggerLabelWrap>
</DropdownButton>
);
Expand All @@ -108,9 +109,3 @@ const OptionLabel = styled('span')`
margin: 0;
}
`;

const FlexContainer = styled('div')`
display: flex;
align-items: center;
gap: ${space(0.75)};
`;
24 changes: 5 additions & 19 deletions static/app/components/codecov/integratedOrgSelector/integratedOrgSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {useCodecovContext} from 'sentry/components/codecov/context/codecovContex
import {LinkButton} from 'sentry/components/core/button/linkButton';
import type {SelectOption} from 'sentry/components/core/compactSelect';
import {CompactSelect} from 'sentry/components/core/compactSelect';
import {Flex} from 'sentry/components/core/layout';
import DropdownButton from 'sentry/components/dropdownButton';
import {IconAdd, IconInfo} from 'sentry/icons';
import {t} from 'sentry/locale';
Expand Down Expand Up @@ -33,7 +34,7 @@ function OrgFooterMessage() {
<Fragment>
<AddIntegratedOrgButton />
<MenuFooterDivider />
<FlexContainer>
<Flex justify="flex-start" gap={space(1)}>
<IconInfo size="sm" style={{margin: '2px 0'}} />
<div>
<FooterInfoHeading>
Expand All @@ -43,7 +44,7 @@ function OrgFooterMessage() {
Ensure you log in to the same <Link to="placeholder">GitHub identity</Link>
</FooterInfoSubheading>
</div>
</FlexContainer>
</Flex>
</Fragment>
);
}
Expand Down Expand Up @@ -95,14 +96,14 @@ export function IntegratedOrgSelector() {
{...triggerProps}
>
<TriggerLabelWrap>
<TriggerFlexContainer>
<Flex justify="flex-start" gap={space(0.75)} align="center">
<IconContainer>
<IconIntegratedOrg />
</IconContainer>
<TriggerLabel>
{integratedOrg || t('Select integrated organization')}
</TriggerLabel>
</TriggerFlexContainer>
</Flex>
</TriggerLabelWrap>
</DropdownButton>
);
Expand Down Expand Up @@ -160,21 +161,6 @@ const MenuFooterDivider = styled('div')`
}
`;

const FlexContainer = styled('div')`
display: flex;
flex-direction: row;
justify-content: flex-start;
gap: ${space(1)};
`;

const TriggerFlexContainer = styled('div')`
display: flex;
flex-direction: row;
justify-content: flex-start;
gap: ${space(0.75)};
align-items: center;
`;

const IconContainer = styled('div')`
flex: 1 0 14px;
height: 14px;
Expand Down
11 changes: 3 additions & 8 deletions static/app/components/codecov/repoPicker/repoSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import styled from '@emotion/styled';
import {Button} from 'sentry/components/core/button';
import type {SelectOption, SingleSelectProps} from 'sentry/components/core/compactSelect';
import {CompactSelect} from 'sentry/components/core/compactSelect';
import {Flex} from 'sentry/components/core/layout';
import DropdownButton from 'sentry/components/dropdownButton';
import Link from 'sentry/components/links/link';
import {IconInfo, IconSync} from 'sentry/icons';
Expand Down Expand Up @@ -118,12 +119,12 @@ export function RepoSelector({onChange, trigger, repository}: RepoSelectorProps)
{...triggerProps}
>
<TriggerLabelWrap>
<FlexContainer>
<Flex align="center" gap={space(0.75)}>
<IconContainer>
<IconRepository />
</IconContainer>
<TriggerLabel>{defaultLabel}</TriggerLabel>
</FlexContainer>
</Flex>
</TriggerLabelWrap>
</DropdownButton>
);
Expand Down Expand Up @@ -178,12 +179,6 @@ const OptionLabel = styled('span')`
}
`;

const FlexContainer = styled('div')`
display: flex;
align-items: center;
gap: ${space(0.75)};
`;

const IconContainer = styled('div')`
flex: 1 0 14px;
height: 14px;
Expand Down
12 changes: 4 additions & 8 deletions static/app/components/events/eventAttachments.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
useFetchEventAttachments,
} from 'sentry/actionCreators/events';
import {LinkButton} from 'sentry/components/core/button/linkButton';
import {Flex} from 'sentry/components/core/layout';
import EventAttachmentActions from 'sentry/components/events/eventAttachmentActions';
import FileSize from 'sentry/components/fileSize';
import LoadingError from 'sentry/components/loadingError';
Expand Down Expand Up @@ -139,9 +140,10 @@ function EventAttachmentsContent({
>
{attachments.map(attachment => (
<Fragment key={attachment.id}>
<FlexCenter>
<Flex align="center">
<Name>{attachment.name}</Name>
</FlexCenter>
</Flex>

<Size>
<FileSize bytes={attachment.size} />
</Size>
Expand Down Expand Up @@ -198,12 +200,6 @@ const StyledPanelTable = styled(PanelTable)`
grid-template-columns: 1fr auto auto;
`;

const FlexCenter = styled('div')`
${p => p.theme.overflowEllipsis};
display: flex;
align-items: center;
`;

const Name = styled('div')`
${p => p.theme.overflowEllipsis};
white-space: nowrap;
Expand Down
49 changes: 20 additions & 29 deletions static/app/components/events/interfaces/spans/newTraceDetailsHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {Fragment} from 'react';
import styled from '@emotion/styled';

import {Flex} from 'sentry/components/core/layout';
import {generateStats} from 'sentry/components/events/opsBreakdown';
import {DividerSpacer} from 'sentry/components/performance/waterfall/miniHeader';
import {t} from 'sentry/locale';
Expand Down Expand Up @@ -31,19 +32,19 @@ function ServiceBreakdown({
}) {
if (!displayBreakdown) {
return (
<BreakDownWrapper>
<BreakDownRow>
<BreakDownWrapper direction="column">
<Flex align="center" justify="space-between">
<div>{t('server side')}</div>
<FlexBox>
<Flex>
<span>{'N/A'}</span>
</FlexBox>
</BreakDownRow>
<BreakDownRow>
</Flex>
</Flex>
<Flex align="center" justify="space-between">
<div>{t('client side')}</div>
<FlexBox>
<Flex>
<span>{'N/A'}</span>
</FlexBox>
</BreakDownRow>
</Flex>
</Flex>
</BreakDownWrapper>
);
}
Expand All @@ -56,21 +57,21 @@ function ServiceBreakdown({
const clientSidePct = 100 - Number(serverSidePct);

return httpDuration ? (
<BreakDownWrapper>
<BreakDownRow>
<BreakDownWrapper direction="column">
<Flex align="center" justify="space-between">
<div>{t('server side')}</div>
<FlexBox>
<Flex>
<Dur>{getDuration(httpDuration, 2, true)}</Dur>
<Pct>{serverSidePct}%</Pct>
</FlexBox>
</BreakDownRow>
<BreakDownRow>
</Flex>
</Flex>
<Flex align="center" justify="space-between">
<div>{t('client side')}</div>
<FlexBox>
<Flex>
<Dur>{getDuration(totalDuration - httpDuration, 2, true)}</Dur>
<Pct>{clientSidePct}%</Pct>
</FlexBox>
</BreakDownRow>
</Flex>
</Flex>
</BreakDownWrapper>
) : null;
}
Expand Down Expand Up @@ -151,18 +152,8 @@ const Pct = styled('div')`
font-variant-numeric: tabular-nums;
`;

const FlexBox = styled('div')`
display: flex;
`;

const BreakDownWrapper = styled(FlexBox)`
flex-direction: column;
const BreakDownWrapper = styled(Flex)`
padding: ${space(2)};
`;

const BreakDownRow = styled(FlexBox)`
align-items: center;
justify-content: space-between;
`;

export default TraceViewHeader;
9 changes: 3 additions & 6 deletions static/app/components/group/times.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {Fragment} from 'react';
import styled from '@emotion/styled';

import {Flex} from 'sentry/components/core/layout';
import TimeSince from 'sentry/components/timeSince';
import {IconClock} from 'sentry/icons';
import {t} from 'sentry/locale';
Expand All @@ -19,7 +20,7 @@ type Props = {
function Times({lastSeen, firstSeen}: Props) {
return (
<Container>
<FlexWrapper>
<FlexWrapper align="center">
{lastSeen && (
<Fragment>
<StyledIconClock legacySize="11px" />
Expand All @@ -42,12 +43,8 @@ const Container = styled('div')`
min-width: 0; /* flex-hack for overflow-ellipsised children */
`;

const FlexWrapper = styled('div')`
const FlexWrapper = styled(Flex)`
Copy link
Member

@scttcper scttcper Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personally think this is worse

Copy link
Member

@scttcper scttcper Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also it changes how it displays since overflowEllipsis is overriding display flex

Copy link
Member Author

@JonasBa JonasBa Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't disagree, but I think it suffers from the same problem as inline styles (which are somewhat growing in popularity across the codebase).

I will try and modify these styled usages to use TextOverflow (at least in this case, it should help)

Copy link
Member Author

@JonasBa JonasBa Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reverted the change locally, to make sure I don't break anything, and I'm either not testing this correctly, or it doesn't even work in it's current state.

CleanShot.2025-06-18.at.14.02.09.mp4

${p => p.theme.overflowEllipsis}

/* The following aligns the icon with the text, fixes bug in Firefox */
display: flex;
align-items: center;
`;

const StyledIconClock = styled(IconClock)`
Expand Down
3 changes: 0 additions & 3 deletions static/app/utils/discover/styles.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ export const BarContainer = styled('div')`
`;

export const FlexContainer = styled('div')`
display: flex;
align-items: center;
justify-content: flex-end;
width: 100%;
`;
Copy link
Member

@natemoo-re natemoo-re Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a replacement here?

Copy link
Member Author

@JonasBa JonasBa Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah no, I actually want to revert this one, and possibly just rename the component given that it's exported


Expand Down
Loading
Loading