Skip to content

core: add 0 case to responsive props #101440

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

Closed
wants to merge 2 commits into from
Closed

core: add 0 case to responsive props #101440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
99678a6
core: add 0 case to responsive props
JonasBa Oct 14, 2025
da0bd30
fix tests
JonasBa Oct 14, 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
15 changes: 15 additions & 0 deletions static/app/components/core/layout/container.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,21 @@ Set positioning for absolute/relative layouts:
</Container>
```

#### 0 breakpoint

The `0` breakpoint is a breakpoint that applies from 0 to the smallest defined breakpoint size.

<Storybook.Demo>
<Container display={{'0': 'none', sm: 'block'}}>
This content is hidden below the sm breakpoint
</Container>
</Storybook.Demo>
```jsx
<Container display={{'0': 'none', sm: 'block'}}>
This content is hidden below the sm breakpoint
</Container>
```

### Grid Integration

The `area` prop supports CSS Grid integration:
Expand Down
11 changes: 8 additions & 3 deletions static/app/components/core/layout/styles.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ describe('useResponsivePropValue', () => {

it('window matches breakpoint = breakpoint value', () => {
const cleanup = setupMediaQueries({
0: false,
xs: true,
sm: true,
md: true,
Expand Down Expand Up @@ -183,6 +184,7 @@ describe('useActiveBreakpoint', () => {
// by doing max-width and min-width and essentially establishing a min value.
it('returns xs as fallback when no breakpoints match', () => {
const cleanup = setupMediaQueries({
0: false,
xs: false,
sm: false,
md: false,
Expand All @@ -194,12 +196,13 @@ describe('useActiveBreakpoint', () => {
wrapper: createWrapper(),
});

expect(result.current).toBe('xs');
expect(result.current).toBe('0');
cleanup();
});

it('returns the largest matching breakpoint', () => {
const cleanup = setupMediaQueries({
0: false,
xs: true,
sm: true,
md: true,
Expand All @@ -224,7 +227,8 @@ describe('useActiveBreakpoint', () => {
});

// Should create media queries for all breakpoints (in reverse order)
expect(matchMediaSpy).toHaveBeenCalledTimes(5);
expect(matchMediaSpy).toHaveBeenCalledTimes(6);
expect(matchMediaSpy).toHaveBeenCalledWith(`(min-width: 0px)`);
expect(matchMediaSpy).toHaveBeenCalledWith(`(min-width: ${theme.breakpoints.xl})`);
expect(matchMediaSpy).toHaveBeenCalledWith(`(min-width: ${theme.breakpoints.lg})`);
expect(matchMediaSpy).toHaveBeenCalledWith(`(min-width: ${theme.breakpoints.md})`);
Expand All @@ -234,6 +238,7 @@ describe('useActiveBreakpoint', () => {

it('uses correct breakpoint order (largest first)', () => {
const cleanup = setupMediaQueries({
0: false,
xs: true,
sm: true,
md: true,
Expand Down Expand Up @@ -341,7 +346,7 @@ describe('useActiveBreakpoint', () => {
);

// Sets up listeners for all breakpoints
expect(addEventListenerSpy).toHaveBeenCalledTimes(5);
expect(addEventListenerSpy).toHaveBeenCalledTimes(6);
unmount();
// Removes listeners for all breakpoints
expect(abortController.abort).toHaveBeenCalledTimes(1);
Expand Down
34 changes: 24 additions & 10 deletions static/app/components/core/layout/styles.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,29 +43,43 @@ export function rc<T>(
if (first) {
first = false;
return css`
@media (min-width: ${theme.breakpoints[breakpoint]}),
(max-width: ${theme.breakpoints[breakpoint]}) {
@media (min-width: ${getBreakpoint(breakpoint, theme)}),
(max-width: ${getBreakpoint(breakpoint, theme)}) {
Copy link
Contributor

@cursor cursor bot Oct 14, 2025

Choose a reason for hiding this comment

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

Bug: Zero Breakpoint Generates Invalid Media Query

When '0' is the first breakpoint in a responsive value, the generated media query includes (max-width: 0). This incorrectly limits the styles to only apply at exactly 0px width, preventing the '0' breakpoint from covering the intended range of small screen sizes.

Fix in Cursor Fix in Web

${property}: ${resolver ? resolver(v, breakpoint, theme) : (v as string)};
}
`;
}

return css`
@media (min-width: ${theme.breakpoints[breakpoint]}) {
@media (min-width: ${getBreakpoint(breakpoint, theme)}) {
${property}: ${resolver ? resolver(v, breakpoint, theme) : (v as string)};
}
`;
}).filter(Boolean)}
`;
}

const BREAKPOINT_ORDER: readonly Breakpoint[] = ['xs', 'sm', 'md', 'lg', 'xl'];
function getBreakpoint(breakpoint: Breakpoint, theme: Theme) {
if (breakpoint === '0') {
Copy link
Member

@JPeer264 JPeer264 Oct 14, 2025

Choose a reason for hiding this comment

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

l: not sure if '0' is intuitive. IMO default or base are more descriptive

Copy link
Member Author

@JonasBa JonasBa Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

What would you expect the behavior of display={{default: 'none', xs: 'block'}} to be?

In the current case with a 0, it means

 0 -> xs = none, then block.

But a default might be misunderstood in the sense that it applies to all unspecified breakpoints, which it does not, and I don't think it should

Copy link
Member

@JPeer264 JPeer264 Oct 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

But a default might be misunderstood in the sense that it applies to all unspecified breakpoints, which it does not, and I don't think it should

Valid point.

After a second thought, when I first used it I actually thought xs is 0-500, since I thought it is following the mobile first principle. So I'm also not sure if 0 would solve it entirely.

Copy link
Collaborator

@TkDodo TkDodo Oct 15, 2025

Choose a reason for hiding this comment

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

I think the biggest confusion might come from the fact that we’re trying to express ranges with a single value. Usually, xs has a single value, e.g. on gap=“xs” we know that it is a certain amount of px.

But on responsive props, xs actually means “xs and above”, or, if we have “md” also defined, it’s “from xs to md”.

I also couldn’t find good documentation on this topic; there is a bit on Container and an example on Stack but that’s about it. I think we should have a dedicated page about it like we have for “Layout Composition”.

That said, I also find 0 adds more confusion (naming wise). An explicit naming would probably be ”<xs”, as in, “smaller than xs”, but I’m not sure if we should go that route 😅.

Copy link
Member Author

@JonasBa JonasBa Oct 15, 2025 •
edited by TkDodo
Loading

Choose a reason for hiding this comment

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

But on responsive props, xs actually means “xs and above”, or, if we have “md” also defined, it’s “from xs to md”.

Yeah, this has been a tradeoff in allowing the min breakpoints to be specified as opposed to having to specify all. Regardless, we would have always probably had this issue in one way or another where we would need to capture > max or < min.

I also couldn’t find good documentation on this topic; there is a bit on Container and an example on Stack but that’s about it. I think we should have a dedicated page about it like we have for “Layout Composition”.
Yes, I can add that.

That said, I also find 0 adds more confusion (naming wise). An explicit naming would probably be ”<xs”, as in, “smaller than xs”, but I’m not sure if we should go that route 😅.

An alternative could also be to define a 2xs breakpoint that does the same thing as what 0 does now, which might be better than 0. I don't like using < operator as that might create confusion if we use {{'<xs'..., 'md': ...}} which would effectively cause <xs to span from 0 to md

Copy link
Member

@JPeer264 JPeer264 Oct 16, 2025

Choose a reason for hiding this comment

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

2xs sounds great

return '0px';
}
return theme.breakpoints[breakpoint];
}

const BREAKPOINT_ORDER: ReadonlyArray<Breakpoint | '0'> = [
'0',
'xs',
'sm',
'md',
'lg',
'xl',
];

// We alias None -> 0 to make it slighly more terse and easier to read.
export type RadiusSize = keyof DO_NOT_USE_ChonkTheme['radius'];
export type SpacingSize = keyof Theme['space'];
export type Border = keyof Theme['tokens']['border'];
export type Breakpoint = keyof Theme['breakpoints'];
export type Breakpoint = keyof Theme['breakpoints'] | '0';

/**
* Prefer using padding or gap instead.
Expand Down Expand Up @@ -257,12 +271,12 @@ export function useActiveBreakpoint(): Breakpoint {

queries.push({
breakpoint: bp,
query: window.matchMedia(`(min-width: ${theme.breakpoints[bp]})`),
query: window.matchMedia(`(min-width: ${getBreakpoint(bp, theme)})`),
});
}

return queries;
}, [theme.breakpoints]);
}, [theme]);
Copy link
Contributor

@cursor cursor bot Oct 14, 2025

Choose a reason for hiding this comment

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

Bug: Breakpoint Zero Bug and useMemo Dependency Issue

The '0' breakpoint now generates a (min-width: 0) media query, which incorrectly matches all screen sizes and causes it to always be active, disrupting responsive behavior. Additionally, the useMemo hook's dependency array changed from [theme.breakpoints] to [theme], leading to unnecessary re-computation of media queries when other theme properties change, impacting performance.

Fix in Cursor Fix in Web


const subscribe = useCallback(
(onStoreChange: () => void) => {
Expand Down Expand Up @@ -303,7 +317,7 @@ function findLargestBreakpoint(
return query.breakpoint;
}

// Since we use min width, the only remaining breakpoint that we might have missed is <xs,
// in which case we return xs, which is in line with behavior of rc() function.
return 'xs';
// Since we use min width, the only remaining breakpoint that we might have missed is <0,
// in which case we return 0, which is in line with behavior of rc() function.
return '0';
}
Loading