Skip to content
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

chore(responsive): add xxs breakpoint prop #8041

Merged
merged 1 commit into from
Oct 25, 2023
Merged

chore(responsive): add xxs breakpoint prop #8041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 6 additions & 4 deletions packages/calcite-components/src/utils/responsive.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ describe("getBreakpoints()", () => {
document.head.innerHTML = `
<style>
:root {
--calcite-app-breakpoint-width-lg: 1000px;
--calcite-app-breakpoint-width-md: 100px;
--calcite-app-breakpoint-width-sm: 10px;
--calcite-app-breakpoint-width-xs: 1px;
--calcite-app-breakpoint-width-lg: 10000px;
--calcite-app-breakpoint-width-md: 1000px;
--calcite-app-breakpoint-width-sm: 100px;
--calcite-app-breakpoint-width-xs: 10px;
--calcite-app-breakpoint-width-xxs: 1px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Breakpoints as CSS Custom Props don't actually work. They can be queried by JS but this is not the ideal way to retrieve these variables. These tokens are also provided by calcite-design-tokens as CSS custom props. However the values do not align.

--calcite-app-breakpoint-width-lg: 1440px; /* min-width size */
  --calcite-app-breakpoint-width-md: 1152px; /* min-width size */
  --calcite-app-breakpoint-width-sm: 768px; /* min-width size */
  --calcite-app-breakpoint-width-xs: 476px; /* min-width size */
  --calcite-app-breakpoint-width-xxs: 320px;

Or as SCSS variables

$breakpoint-width-lg: 1440px; // min-width size
$breakpoint-width-md: 1152px; // min-width size
$breakpoint-width-sm: 768px; // min-width size
$breakpoint-width-xs: 476px; // min-width size
$breakpoint-width-xxs: 320px; // Small handheld devices and mini-windows

The new PR (8044) also provides the same breakpoints now as a JS object

import tokenJSON from "@esri/calcite-design-tokens/js";

const breakpointXxs = tokenJSON.core.breakpoint.width.xxs.value;

or as tree-shakeable named exports

import { CoreBreakpointWidthXxs } from "@esri/calcite-design-tokens/es6";

Do we still need this PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Breakpoints as CSS Custom Props don't actually work.

Can you provide more info on this? I'm not sure I follow.

They can be queried by JS but this is not the ideal way to retrieve these variables. These tokens are also provided by calcite-design-tokens as CSS custom props. However the values do not align.

There are a few factors on why we get the breakpoints by evaluating the CSS at runtime:

  1. no JS object at the time
  2. unclear if we were allowing users to override breakpoints via CSS props or not
  3. wanted to avoid introducing a third way to reference tokens within the code base

Depending on 2, we could revisit this module's implementation (in a follow-up PR) and set some guidelines for 3.

These breakpoints do not align with designer approved breakpoints

FWIW, this test cover looking up the breakpoints CSS props and isn't meant to match the actual breakpoint values.

Copy link
Contributor

@alisonailea alisonailea Oct 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Can you provide more info on this? I'm not sure I follow.

https://bholmes.dev/blog/alternative-to-css-variable-media-queries/

  1. unclear if we were allowing users to override breakpoints via CSS props or not

While a consumer could override any custom prop including --calcite-app-breakpoint-width-lg it won't do them any good because CSS will not use custom props in media breakpoint queries as shown in the link above. This can be accomplished via SCSS but that would rely on the SCSS var $breakpoint-width-lg before compiling.

FWIW, this test cover looking up the breakpoints CSS props and isn't meant to match the actual breakpoint values.

Got it! Thanks for the clarification.

Copy link
Contributor

Choose a reason for hiding this comment

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

Container Style Queries can be used to set breakpoint styles with CSS Custom Props but that's currently only available in chrome https://developer.chrome.com/blog/style-queries/

Copy link
Member Author

Choose a reason for hiding this comment

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

While a consumer could override any custom prop including --calcite-app-breakpoint-width-lg it won't do them any good because CSS will not use custom props in media breakpoint queries as shown in the link above. This can be accomplished via SCSS but that would rely on the SCSS var $breakpoint-width-lg before compiling.
Container Style Queries can be used to set breakpoint styles with CSS Custom Props but that's currently only available in chrome https://developer.chrome.com/blog/style-queries/

Ah, that's right. Thanks for catching that and clarifying.

The responsive util does pick up breakpoint CSS props overrides at load time, but we would still have a disconnect in a few places (namely stylesheet and conditional rendering) because we can't use style queries/environment variables yet.

I'll update the util in a follow-up PR to use the JS exports instead. ✨🔧✨

}
</style>
`;
Expand All @@ -22,6 +23,7 @@ describe("getBreakpoints()", () => {
medium: toBeInteger(),
small: toBeInteger(),
xsmall: toBeInteger(),
xxsmall: toBeInteger(),
},
});
});
Expand Down
2 changes: 2 additions & 0 deletions packages/calcite-components/src/utils/responsive.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export interface Breakpoints {
medium: number;
small: number;
xsmall: number;
xxsmall: number;
};
}

Expand Down Expand Up @@ -35,6 +36,7 @@ export async function getBreakpoints(): Promise<Breakpoints> {
medium: breakpointTokenToNumericalValue(rootStyles, "--calcite-app-breakpoint-width-md"),
small: breakpointTokenToNumericalValue(rootStyles, "--calcite-app-breakpoint-width-sm"),
xsmall: breakpointTokenToNumericalValue(rootStyles, "--calcite-app-breakpoint-width-xs"),
xxsmall: breakpointTokenToNumericalValue(rootStyles, "--calcite-app-breakpoint-width-xxs"),
},
});
});
Expand Down
1 change: 1 addition & 0 deletions support/dictionaries/dict-calcite-design-system.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ tibt

## sizing
xsmall
xxsmall

## svg
svgs
Expand Down
Loading