Skip to content

feat(ui): chonkify segmentedControl #89589

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 14 commits into from
Apr 15, 2025
Merged

feat(ui): chonkify segmentedControl #89589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
881a91b
ref: simplify segmentedControl types
TkDodo Apr 11, 2025
193d398
ref: remove uncontrolled version of segmentedControl
TkDodo Apr 11, 2025
7516449
fix: no-unnecessary-type-assertion
TkDodo Apr 11, 2025
339f364
feat: first attempt at chonkifying segmentedControl
TkDodo Apr 11, 2025
0d625a2
feat: focusRing
TkDodo Apr 11, 2025
0f3cf1a
fix: fallback focus ring for browsers where :has is not supported
TkDodo Apr 11, 2025
523e8c8
Merge remote-tracking branch 'origin/master' into tkdodo/feat/chonkif…
TkDodo Apr 11, 2025
0b3a610
Merge remote-tracking branch 'origin/master' into tkdodo/feat/chonkif…
TkDodo Apr 15, 2025
bcd0d1b
simplify dom structure in chonk
TkDodo Apr 15, 2025
02d449c
style selected item
TkDodo Apr 15, 2025
da1241a
Merge remote-tracking branch 'origin/master' into tkdodo/feat/chonkif…
TkDodo Apr 15, 2025
4469f16
fix: disabled styles
TkDodo Apr 15, 2025
e74f1d0
fix: borders
TkDodo Apr 15, 2025
4c010d8
chore: move z-index
TkDodo Apr 15, 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
8 changes: 5 additions & 3 deletions static/app/components/core/button/index.chonk.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ function chonkSizeMapping(size: ButtonProps['size']): ChonkButtonSize {
}

export function getChonkButtonStyles(
p: ButtonProps & {theme: DO_NOT_USE_ChonkTheme}
p: Pick<ButtonProps, 'size' | 'priority' | 'busy' | 'disabled' | 'borderless'> & {
theme: DO_NOT_USE_ChonkTheme;
}
Comment on lines 53 to +56
Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed the interface here to be as minimal as possible - only what gets really used, so that I can re-use it in the segmentgedControl

): StrictCSSObject {
const type = chonkPriorityToType(p.priority);
const size = chonkSizeMapping(p.size);
Expand Down Expand Up @@ -150,7 +152,7 @@ export function getChonkButtonStyles(
},
},

'&:active, &[aria-expanded="true"]': {
'&:active, &[aria-expanded="true"], &[aria-checked="true"]': {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

segmentedControl is a checkbox and thus sets aria-checked for “active” elements

'&::after': {
transform: 'translateY(0px)',
},
Expand All @@ -159,7 +161,7 @@ export function getChonkButtonStyles(
},
},

'&:disabled': {
'&:disabled, &[aria-disabled="true"]': {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’ve set aria-disabled for the element in the segmentedControl that receives styling, so I need to match it here, too.

'&::after': {
transform: 'translateY(0px)',
},
Expand Down
102 changes: 102 additions & 0 deletions static/app/components/core/segmentedControl/index.chonk.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import type {DO_NOT_USE_ChonkTheme} from '@emotion/react';

import {getChonkButtonStyles} from 'sentry/components/core/button/index.chonk';
import type {FormSize} from 'sentry/utils/theme';
import {chonkStyled} from 'sentry/utils/theme/theme.chonk';

export type Priority = 'default' | 'primary';

export const ChonkStyledGroupWrap = chonkStyled('div')<{
priority: Priority;
size: FormSize;
}>`
position: relative;
display: inline-grid;
grid-auto-flow: column;
min-width: 0;

${p => p.theme.form[p.size]}

& > label:first-child {
border-top-right-radius: 0;
border-bottom-right-radius: 0;
border-right: 0;
}

& > label:not(:first-child):not(:last-child) {
border-radius: 0;
}

& > label:last-child {
border-top-left-radius: 0;
border-bottom-left-radius: 0;
}

/* don't turn off border if the 2nd element is also the last element */
& > label:last-child:not(:nth-child(2)) {
border-left: 0;
}
`;

export const ChonkStyledSegmentWrap = chonkStyled('label')<{
isSelected: boolean;
priority: Priority;
size: FormSize;
isDisabled?: boolean;
}>`
position: relative;
display: flex;
align-items: center;
margin: 0;
cursor: ${p => (p.isDisabled ? 'default' : 'pointer')};
min-height: 0;
min-width: 0;
z-index: ${p => (p.isSelected ? 1 : undefined)};

${p => p.theme.buttonPadding[p.size]}
font-weight: ${p => p.theme.fontWeightNormal};

${p => ({...getChonkButtonStyles({...p, disabled: p.isDisabled, priority: p.isSelected && p.priority === 'primary' ? 'primary' : 'default'})})}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

took me a while to figure out why disabled styles don’t work. disabled vs isDisabled, and no typescript errors because the prop is optional and there is no excess property checking 😭

Copy link
Member

Choose a reason for hiding this comment

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

Oh my god... we really need to delete these dual props, this is a waste of time + having isDisabled and disabled is just confusing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we use isDisabled a lot because of “boolean props should start with is”, which is okay, but then we have disabled because the html property is disabled and this gets forwarded to inputs :/

let’s put it on the list to decide for one


&:has(input:focus-visible) {
${p => p.theme.focusRing};

/* Hide fallback ring when :has works */
span {
box-shadow: none !important;
}
}

/* Fallback ring (for Firefox, where :has doesn't work) */
input:focus-visible + span {
${({theme}) => theme.focusRing};
}
`;

export const ChonkStyledVisibleLabel = chonkStyled('span')<{
isSelected: boolean;
priority: Priority;
}>`
${p => p.theme.overflowEllipsis}
user-select: none;
font-weight: ${p => p.theme.fontWeightNormal};
text-align: center;
color: ${p => getTextColor(p)};
`;

function getTextColor({
isSelected,
priority,
theme,
}: {
isSelected: boolean;
priority: Priority;
theme: DO_NOT_USE_ChonkTheme;
isDisabled?: boolean;
}) {
if (isSelected) {
return priority === 'default' ? theme.colors.blue500 : undefined;
}

return theme.subText;
}
34 changes: 27 additions & 7 deletions static/app/components/core/segmentedControl/index.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import storyBook from 'sentry/stories/storyBook';
export default storyBook('SegmentedControl', story => {
story('Controlled Value', () => {
const [value, setValue] = useState('two');
const [value2, setValue2] = useState('one');
return (
<Fragment>
<p>
Expand All @@ -19,11 +20,30 @@ export default storyBook('SegmentedControl', story => {
manually.
</p>
<p>selected={value}</p>
<SegmentedControl value={value} onChange={setValue}>
<SegmentedControl.Item key="one">One</SegmentedControl.Item>
<SegmentedControl.Item key="two">Two</SegmentedControl.Item>
<SegmentedControl.Item key="three">Three</SegmentedControl.Item>
</SegmentedControl>
<SideBySide>
<SegmentedControl value={value} onChange={setValue} priority="default">
<SegmentedControl.Item key="one">One</SegmentedControl.Item>
<SegmentedControl.Item key="two">Two</SegmentedControl.Item>
<SegmentedControl.Item key="three">Three</SegmentedControl.Item>
</SegmentedControl>
<SegmentedControl value={value} onChange={setValue} priority="primary">
<SegmentedControl.Item key="one">One</SegmentedControl.Item>
<SegmentedControl.Item key="two">Two</SegmentedControl.Item>
<SegmentedControl.Item key="three">Three</SegmentedControl.Item>
</SegmentedControl>
</SideBySide>

<p>selected={value2}</p>
<SideBySide>
<SegmentedControl value={value2} onChange={setValue2} priority="default">
<SegmentedControl.Item key="one">One</SegmentedControl.Item>
<SegmentedControl.Item key="two">Two</SegmentedControl.Item>
</SegmentedControl>
<SegmentedControl value={value2} onChange={setValue2} priority="primary">
<SegmentedControl.Item key="one">One</SegmentedControl.Item>
<SegmentedControl.Item key="two">Two</SegmentedControl.Item>
</SegmentedControl>
</SideBySide>
</Fragment>
);
});
Expand Down Expand Up @@ -90,13 +110,13 @@ export default storyBook('SegmentedControl', story => {
const [value, setValue] = useState('two');
return (
<SegmentedControl value={value} onChange={setValue}>
<SegmentedControl.Item key="one" tooltip="One">
<SegmentedControl.Item key="one" {...props}>
One
</SegmentedControl.Item>
<SegmentedControl.Item key="two" {...props}>
Two
</SegmentedControl.Item>
<SegmentedControl.Item key="three" tooltip="Three">
<SegmentedControl.Item key="three" {...props}>
Three
</SegmentedControl.Item>
</SegmentedControl>
Expand Down
Loading
Loading