-
Notifications
You must be signed in to change notification settings - Fork 2k
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
SPT: Isolate TemplateSelectorControl component #38445
Changes from all commits
b6207a8
2b197af
e06b61a
c2bbed8
b73066c
f4adda3
ba960df
7b3639e
40b73e8
be80333
36920fc
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 |
---|---|---|
@@ -0,0 +1,190 @@ | ||
@import '~@wordpress/base-styles/colors'; // @TODO: Remove once https://github.com/WordPress/gutenberg/pull/19159 is merged and released. | ||
@import '~@wordpress/base-styles/variables'; | ||
@import './shared.scss'; | ||
|
||
// Preview positioning | ||
$preview-max-width: 960px; | ||
$preview-right-margin: 24px; | ||
$preview-left-sidebar-reduced: calc( 30% + #{$admin-sidebar-width-collapsed} ); | ||
$preview-left-sidebar-full: calc( 30% + #{$admin-sidebar-width} ); | ||
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. Will 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. idk 😅 -- But all I did in f4adda3 is remove two SASS var definitions with two other ones that I'm importing from 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.
It shouldn't indeed and sounds fair! :-) Just good to test both scenarios anyway always when CSS is touched for anything FSE plugin related. |
||
|
||
$template-large-preview-title-height: 120px; | ||
|
||
// Template Selector Preview | ||
.template-selector-preview { | ||
@media screen and ( max-width: 659px ) { | ||
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 we have mixins or variables available in Gutenberg for these screen sizes? 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. Couldn't find any (tried grepping 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. Gotcha — I guess in some follow up some day we could re-use breakpoints from |
||
display: none; | ||
} | ||
|
||
@media screen and ( min-width: 783px ) { | ||
width: calc( 70% - 50px ); | ||
} | ||
|
||
@media screen and ( min-width: 660px ) { | ||
&.is-blank-preview { | ||
align-items: center; | ||
display: flex; | ||
justify-content: center; | ||
} | ||
} | ||
|
||
@media screen and ( min-width: 1648px ) { | ||
right: unset; | ||
left: calc( | ||
#{$preview-left-sidebar-full} + ( | ||
100vw - #{$preview-left-sidebar-full} - #{$preview-max-width} - #{$preview-right-margin} | ||
) / 2 | ||
); | ||
body.folded & { | ||
left: calc( | ||
#{$preview-left-sidebar-reduced} + ( | ||
100vw - #{$preview-left-sidebar-reduced} - #{$preview-max-width} - #{$preview-right-margin} | ||
) / 2 | ||
); | ||
} | ||
} | ||
|
||
position: fixed; | ||
top: 111px; | ||
bottom: 24px; | ||
right: $preview-right-margin; | ||
width: calc( 80% - 50px ); | ||
background: $template-selector-empty-background; | ||
border-radius: 2px; | ||
overflow-x: hidden; | ||
overflow-y: auto; | ||
box-shadow: 0 2px 2px 0 rgba( 0, 0, 0, 0.14 ), 0 3px 1px -2px rgba( 0, 0, 0, 0.12 ), | ||
0 1px 5px 0 rgba( 0, 0, 0, 0.2 ); | ||
|
||
.edit-post-visual-editor { | ||
margin: 0; | ||
padding: 0; | ||
} | ||
|
||
.editor-styles-wrapper { | ||
.template-selector__preview-offset-correction { | ||
position: relative; | ||
top: $template-large-preview-title-height; | ||
} | ||
|
||
.editor-post-title { | ||
transform-origin: top left; | ||
width: 960px; | ||
display: block; | ||
position: absolute; | ||
top: 0; | ||
} | ||
|
||
.editor-post-title, | ||
.editor-post-title__block, | ||
.editor-post-title__input { | ||
padding-top: 0; | ||
padding-bottom: 0; | ||
margin-top: 0; | ||
margin-bottom: 0; | ||
.editor-post-title__input { | ||
margin: 0; | ||
padding: 0; | ||
height: $template-large-preview-title-height; | ||
line-height: $template-large-preview-title-height; | ||
overflow: hidden; | ||
resize: none; | ||
} | ||
} | ||
} | ||
} | ||
|
||
body:not( .is-fullscreen-mode ) { | ||
.template-selector-preview { | ||
@media screen and ( min-width: 783px ) { | ||
width: calc( 70% - #{$admin-sidebar-width-collapsed + ( 24px * 2 )} ); | ||
} | ||
|
||
@media screen and ( min-width: 961px ) { | ||
width: calc( 70% - #{$admin-sidebar-width} ); | ||
} | ||
} | ||
@media screen and ( min-width: 783px ) { | ||
&.folded .template-selector-preview { | ||
width: calc( 70% - #{$admin-sidebar-width-collapsed + ( 24px * 2 )} ); | ||
} | ||
&:not( .folded ):not( .auto-fold ) .template-selector-preview { | ||
width: calc( 70% - #{$admin-sidebar-width} ); | ||
} | ||
} | ||
} | ||
|
||
.template-selector__preview-placeholder { | ||
color: var( --color-text-subtle ); | ||
font-size: 15px; | ||
font-weight: 400; | ||
} | ||
|
||
// Preview adjustments. | ||
// Tweak styles which are inside of the preview container. | ||
.block-editor-block-preview__container, | ||
.template-selector-preview { | ||
max-width: $preview-max-width; | ||
|
||
.editor-styles-wrapper { | ||
.wp-block { | ||
width: 100%; | ||
} | ||
|
||
.wp-block[data-align='wide'] { | ||
//max-width: 800px; | ||
} | ||
|
||
// `core/cover` | ||
.wp-block[data-type='core/cover'][data-align='full'] { | ||
margin: 0; | ||
.wp-block-cover { | ||
padding: 0; | ||
} | ||
} | ||
|
||
// `core/columns` | ||
.wp-block-columns | ||
> .editor-inner-blocks | ||
> .editor-block-list__layout | ||
> [data-type='core/column'] { | ||
//margin-left: -14px; | ||
//margin-right: -14px; | ||
|
||
& > .editor-block-list__block-edit > div > .block-core-columns > .editor-inner-blocks { | ||
margin-top: 0; | ||
margin-bottom: 0; | ||
} | ||
} | ||
|
||
.block-editor-block-list__block { | ||
&[data-align='full'] { | ||
margin: 0; | ||
} | ||
|
||
.block-editor-block-list__block-edit { | ||
@media screen and ( min-width: 600px ) { | ||
margin: 0; | ||
} | ||
} | ||
} | ||
|
||
// Fix upstream: https://github.com/WordPress/gutenberg/pull/17202. | ||
.block-editor-block-list__layout, | ||
.block-editor-block-list__block { | ||
padding: inherit; | ||
} | ||
} | ||
} | ||
|
||
// Set full height to preview container to inherits styles defined for themes. | ||
.template-selector-preview .components-disabled, | ||
.template-selector-preview .edit-post-visual-editor, | ||
.template-selector-item__preview-wrap .components-disabled, | ||
.template-selector-item__preview-wrap .edit-post-visual-editor { | ||
height: 100%; | ||
|
||
.editor-styles-wrapper { | ||
height: 100%; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
$template-selector-border-color: #e2e4e7; | ||
$template-selector-border-color-selected: #555d66; | ||
$template-selector-border-color-active: #00a0d2; | ||
$template-selector-border-color-hover: #c9c9ca; | ||
$template-selector-empty-background: #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. While it's ok to have them here, should these variables be pulled in from Gutenberg base styles or color studio? I.e.: $template-selector-empty-background: $white; |
||
$template-selector-modal-offset-bottom: 25px; | ||
$template-selector-modal-offset-right: 32px; | ||
$template-selector-blank-template-mobile-height: 70px; | ||
|
||
@mixin screen-reader-text() { | ||
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. Isn't this mixin either in Calypso or Gutenberg base styles already? Could we import it from there? 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 couldn't find it in Calypso upon a quick search. It appears to be in Gutenberg, https://github.com/WordPress/gutenberg/blob/bfd1117846c7802d62dbba046c27968d961781e7/packages/block-editor/src/components/responsive-block-control/style.scss#L1-L12, so we might be able to use that -- unless there's more implicit dependencies (like this: WordPress/gutenberg#19159). Overall, this PR is just moving around pre-existing styling; I'd like to limit changes that go beyond that in order to unblock it soon 😬 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 might actually want to use that entire component rather than just its styling.) 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 moving things around in this PR makes sense. 👍 |
||
border: 0; | ||
clip: rect( 1px, 1px, 1px, 1px ); | ||
clip-path: inset( 50% ); | ||
height: 1px; | ||
margin: -1px; | ||
overflow: hidden; | ||
padding: 0; | ||
position: absolute; | ||
width: 1px; | ||
word-wrap: normal !important; | ||
} |
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.
Since WordPress/gutenberg#19159 is an issue at the moment, you could also import the whole set via this file:
wp-calypso/client/assets/stylesheets/gutenberg-base-styles.scss
Lines 1 to 6 in f2b1724
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 pushed be80333 which seems to be enough.