-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Try G2 #19344
Try G2 #19344
Changes from all commits
c02489b
889b891
334a925
9f2a4c8
9cf6785
3b83cff
962e343
b4bc4eb
d146aa4
d5d8a0a
c36998c
c08e520
91d4e51
9d5ae41
c13f756
6dffd8b
7c0b54e
89da294
90f4b32
87980ea
9e37e63
f04ff76
b75af39
5602b7a
baaf2ad
efa7f91
da73af8
2d61458
dd732d0
f27376d
96d3503
06c9eb7
5d9d315
368a174
1263f64
fd0f679
c704c74
f4becd7
20e9d9c
dd1c844
2d4af41
601d5dd
a23c5d4
0782246
59c4dc6
047b2a2
79aa5cf
10705ed
6df212a
52d6b76
6363ff3
22bd2d8
6daf068
c745cec
819989b
e986412
12756c4
6d946a9
41b7eaf
9c0e0b3
ead309d
b1f3a9d
91fdca1
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 |
---|---|---|
|
@@ -25,6 +25,15 @@ $light-gray-200: #f3f4f5; | |
$light-gray-100: #f8f9f9; | ||
$white: #fff; | ||
|
||
// G2 colors. | ||
$dark-gray-primary: #1e1e1e; | ||
$medium-gray-text: #757575; | ||
$light-gray-secondary: #ccc; | ||
$light-gray-tertiary: #e7e8e9; | ||
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. Do you think we should replace all shared of grays (all gray variables) with these four variables? 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. Yes, but I think it's also an opportunity to reduce the amount of grays we have. The wide range has not proven as useful in practice, given that they are intended to colorize the UI, vs. for example colorize a theme. |
||
$theme-color: theme(button); | ||
$blue-medium-focus: #007cba; // @todo: Currently being used as "spot" color. Needs to be considered in context of themes. | ||
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. should we rename this $theme-color-focus and just use SASS functions (tint or shade) to compute it based on 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. Mmmmmaybe. As far as is possible at all, we should try and avoid the SASS coloring function as it can never be as good as a manually chosen shade. However if the resting state color is manually picked, it's probably fine if hover and active states are SASS shaded. |
||
$blue-medium-focus-dark: #fff; | ||
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 white, not blue :) I believe ideally the variable name should contain "blue" but be more about the usage. 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. We could rename both of these to $block-focus and $block-focus-dark? Because that's what they are. 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. #fff for dark? Is this just about the blocks or all focus styles (buttons,...), I'd rather avoid the "block" prefix and maybe replace with something more generic. 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. |
||
|
||
// Dark opacities, for use with light themes. | ||
$dark-opacity-900: rgba(#000510, 0.9); | ||
$dark-opacity-800: rgba(#00000a, 0.85); | ||
|
@@ -82,8 +91,6 @@ $blue-medium-300: #66c6e4; | |
$blue-medium-200: #bfe7f3; | ||
$blue-medium-100: #e5f5fa; | ||
$blue-medium-highlight: #b3e7fe; | ||
$blue-medium-focus: #007cba; | ||
$blue-medium-focus-dark: #fff; | ||
|
||
// Alert colors. | ||
$alert-yellow: #f0b849; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,12 +123,6 @@ | |
cursor: default; | ||
} | ||
|
||
@mixin button-style__hover { | ||
background-color: $white; | ||
color: $dark-gray-900; | ||
box-shadow: inset 0 0 0 1px $dark-gray-500, inset 0 0 0 2px $white; | ||
} | ||
|
||
@mixin button-style__active() { | ||
outline: none; | ||
background-color: $white; | ||
|
@@ -137,12 +131,10 @@ | |
} | ||
|
||
@mixin button-style__focus-active() { | ||
background-color: $white; | ||
color: $dark-gray-900; | ||
box-shadow: inset 0 0 0 1px $dark-gray-300, inset 0 0 0 2px $white; | ||
box-shadow: 0 0 0 1px color($theme-color); | ||
|
||
// Windows High Contrast mode will show this outline, but not the box-shadow. | ||
outline: 2px solid transparent; | ||
outline: 1px solid transparent; | ||
} | ||
|
||
// Switch. | ||
|
@@ -154,26 +146,17 @@ | |
outline-offset: 2px; | ||
} | ||
|
||
// Formatting Buttons. | ||
@mixin formatting-button-style__hover { | ||
color: $dark-gray-500; | ||
box-shadow: inset 0 0 0 1px $dark-gray-500, inset 0 0 0 2px $white; | ||
} | ||
|
||
@mixin formatting-button-style__active() { | ||
outline: none; | ||
color: $white; | ||
box-shadow: none; | ||
background: $dark-gray-500; | ||
} | ||
|
||
@mixin formatting-button-style__focus() { | ||
box-shadow: inset 0 0 0 1px $dark-gray-500, inset 0 0 0 2px $white; | ||
/** | ||
* Block Toolbar/Formatting Buttons | ||
*/ | ||
|
||
// Windows High Contrast mode will show this outline, but not the box-shadow. | ||
outline: 2px solid transparent; | ||
@mixin block-toolbar-button-style__focus() { | ||
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 have the same concern raise about the previous mixin. Should we just duplicate in Popover? 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. The mixin conversation I feel is worth elevating above this PR. They are intended to help create a single place to define reused code. But at the same time I can see how that goes against the CSS & components mantra. |
||
box-shadow: inset 0 0 0 2px color($theme-color), inset 0 0 0 4px $white; // Inner halo makes this work on top of a toggled button. | ||
outline: 2px solid transparent; // Shown to Windows 10 High Contrast Mode. | ||
} | ||
|
||
|
||
// Tabs, Inputs, Square buttons. | ||
@mixin input-style__neutral() { | ||
box-shadow: 0 0 0 transparent; | ||
|
@@ -231,35 +214,26 @@ | |
} | ||
|
||
@mixin block-style__hover { | ||
background: $light-gray-200; | ||
color: $dark-gray-900; | ||
border-color: $theme-color; | ||
color: $theme-color !important; | ||
} | ||
|
||
@mixin block-style__focus() { | ||
color: $dark-gray-900; | ||
box-shadow: 0 0 0 1px $white, 0 0 0 3px $blue-medium-500; | ||
box-shadow: inset 0 0 0 1px $white, 0 0 0 2px $theme-color; | ||
|
||
// Windows High Contrast mode will show this outline, but not the box-shadow. | ||
outline: 2px solid transparent; | ||
} | ||
|
||
@mixin block-style__is-active() { | ||
color: $dark-gray-900; | ||
box-shadow: inset 0 0 0 2px $dark-gray-500; | ||
color: $white; | ||
background: $dark-gray-primary; | ||
|
||
// Windows High Contrast mode will show this outline, but not the box-shadow. | ||
outline: 2px solid transparent; | ||
outline-offset: -2px; | ||
} | ||
|
||
@mixin block-style__is-active-focus() { | ||
color: $dark-gray-900; | ||
box-shadow: 0 0 0 1px $white, 0 0 0 3px $blue-medium-500, inset 0 0 0 2px $dark-gray-500; | ||
|
||
// Windows High Contrast mode will show this outline, but not the box-shadow. | ||
outline: 4px solid transparent; | ||
outline-offset: -4px; | ||
} | ||
|
||
/** | ||
* Applies editor left position to the selector passed as argument | ||
|
@@ -337,13 +311,14 @@ | |
border-left: 3px solid transparent; | ||
border-right: 3px solid transparent; | ||
border-top: 5px solid; | ||
margin-left: $grid-size-small; | ||
margin-left: $grid-unit-05; | ||
|
||
// This gives the icon space on the right side consistent with the material | ||
// icon standards. | ||
margin-right: 2px; | ||
} | ||
|
||
|
||
/** | ||
* Allows users to opt-out of animations via OS-level preferences. | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,7 @@ | |
display: flex; | ||
justify-content: space-between; | ||
color: $dark-gray-400; | ||
margin-top: $grid-size; | ||
margin-top: $grid-unit-10; | ||
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. Ultimately, I think it's better to avoid relying on this kind of variables that refer to a "value" (10 units). Instead we should prefer more semantic variables ($default-whitespace, $button-size, ...). For example if you use $button-size variable here, it would mean that this value should follow the button size, if the button size changes, this value should change while when rely on "values" like here. There's no specific meaning, it makes maintenance harder. If tomorrow I change the value of this variable, am I assured that it won't break the layout? Just a general thought, nothing to consider specifically for this PR 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'm actually introducing those variables to help enable a grid system that works with chunks of 8 and emphasis on 12 and 24. When used as the only units of spacing, rather than arbitrary pixel numbers, they can help add consistency to the grid. There's more in https://make.wordpress.org/design/2019/10/31/proposal-a-consistent-spacing-system-for-wordpress/, which this is directly based on. |
||
font-size: 12px; | ||
|
||
.block-directory-downloadable-block-info__column { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the same remark about these variables. I think we should avoid using "gray", "blue" in these. Even "dark", "light" I'm not sure about them.
I'd prefer if we use semantic variables. "primary-border-color"... That way when we decide to change the color itself, we won't have to change the variable name. It won't go out of sync.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that point. @pablohoneyhoney any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should change these variable names.