-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Theme: Add density support for surface padding #73215
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.
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
Changes from all commits
e07057d
db45b28
f2c0bf1
a2c55d2
32279f9
fbcb7b0
0d37ed4
79291f9
25ac161
4fe0f86
94848d3
c9c8985
80cbbea
56672d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -558,27 +558,43 @@ | |
| }, | ||
| "description": "Base dimension unit" | ||
| }, | ||
| "Dimension/Semantic/padding-surface-x-small": { | ||
| "Dimension/Semantic/padding-surface-2xs": { | ||
| "value": { | ||
| ".": "8px" | ||
| ".": "4px", | ||
| "compact": "4px", | ||
| "comfortable": "8px" | ||
| }, | ||
| "description": "2x extra small spacing for surfaces" | ||
| }, | ||
| "Dimension/Semantic/padding-surface-xs": { | ||
| "value": { | ||
| ".": "8px", | ||
| "compact": "4px", | ||
| "comfortable": "12px" | ||
| }, | ||
| "description": "Extra small spacing for surfaces" | ||
| }, | ||
| "Dimension/Semantic/padding-surface-small": { | ||
| "Dimension/Semantic/padding-surface-sm": { | ||
| "value": { | ||
| ".": "12px" | ||
| ".": "16px", | ||
| "compact": "12px", | ||
| "comfortable": "20px" | ||
| }, | ||
| "description": "Small spacing for surfaces" | ||
| }, | ||
| "Dimension/Semantic/padding-surface-medium": { | ||
| "Dimension/Semantic/padding-surface-md": { | ||
| "value": { | ||
| ".": "16px" | ||
| ".": "24px", | ||
| "compact": "20px", | ||
| "comfortable": "32px" | ||
| }, | ||
| "description": "Medium spacing for surfaces" | ||
| }, | ||
| "Dimension/Semantic/padding-surface-large": { | ||
| "Dimension/Semantic/padding-surface-lg": { | ||
| "value": { | ||
| ".": "24px" | ||
| ".": "32px", | ||
| "compact": "24px", | ||
| "comfortable": "40px" | ||
| }, | ||
| "description": "Large spacing for surfaces" | ||
| }, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we'll probably need tokens for 32px and 48px padding. 32px is currently used in Dialogs and 48px in DataViews. I guess that also means we'll need a new primitive so that 48px can jump to 56px in comfortable density. |
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -23,6 +23,15 @@ export interface ThemeProviderSettings { | |||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| bg?: string; | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * The density of the theme. If left unspecified, the theme inherits from | ||||||||||||||||||||||||||||||||||
| * the density of the closest `ThemeProvider`, or uses the default density | ||||||||||||||||||||||||||||||||||
| * if there is no inherited density. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @default undefined | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| density?: undefined | 'default' | 'compact' | 'comfortable'; | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+27
to
+34
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a thought, no strong opinion, but would something like this be more intuitive? Personally I think it would be easier to understand when skimming the docs, since it's closer to the CSS mental model.
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, I think there's some advantages to that, particularly around explicitness. However, on the whole, I don't know that it's a change I want to make now:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's fair. Maybe it looks weird to me because all our "physical" components do |
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export interface ThemeProviderProps extends ThemeProviderSettings { | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.