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

Fix cover matrix alignment #28404

Merged
merged 6 commits into from
Jan 22, 2021
Merged

Fix cover matrix alignment #28404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e9e1ab8
Fix cover matrix alignment
ajlende Jan 21, 2021
729d653
Consolidate editor and save styles
ajlende Jan 21, 2021
8b698ab
Override img margin/padding from .wp-block-cover.alignright > *
ajlende Jan 21, 2021
ad72dcf
Fix cover block height in Twenty Twenty
ajlende Jan 21, 2021
e4dee72
Add more rules.
jasmussen Jan 22, 2021
2543962
Move height back.
jasmussen Jan 22, 2021
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
8 changes: 1 addition & 7 deletions packages/block-library/src/cover/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
.wp-block-cover {
position: relative;
height: 100%; // This explicit height rule is necessary for the focal point picker to work.

// Override default cover styles
// because we're not ready yet to show the cover block.
Expand Down Expand Up @@ -43,12 +43,6 @@
.wp-block-cover__placeholder-background-options {
width: 100%;
}

// Fix object-fit when block is full width in the editor.
.wp-block-cover__image-background {
width: inherit;
height: inherit;
}
}

[data-align="left"] > .wp-block-cover,
Expand Down
27 changes: 19 additions & 8 deletions packages/block-library/src/cover/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
background-position: center center;
min-height: 430px;
width: 100%;
Copy link
Contributor

Choose a reason for hiding this comment

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

This rule was moved from style.scss to editor.scss, which makes me nervous. In general, unless it's explicitly for block editor specific shenanigans, like extra divs, adjacent markup, or visuals that appear only in the editor, my instinct is to keep as much of the CSS as possible in style.scss. That way any visual issues you fix on the frontend, you also fix in the editor.

I might move this back, but I need to test a little bit more first.

height: 100%; // This explicit height rule is necessary for the focal point picker to work.
display: flex;
justify-content: center;
align-items: center;
Expand Down Expand Up @@ -167,14 +166,26 @@
width: auto;
}
}
}

.wp-block-cover__image-background,
.wp-block-cover__video-background {
position: absolute;
width: 100%;
height: 100%;
object-fit: cover;
// Extra specificity for in edit mode where other styles would override it otherwise.
img.wp-block-cover__image-background,
video.wp-block-cover__video-background {
Comment on lines +170 to +172
Copy link
Contributor

@ockham ockham Jan 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Do we need the img and video element-based selectors? 😅 I was hoping that moving the selectors into .wp-block-cover would be specific enough.

Suggested change
// Extra specificity for in edit mode where other styles would override it otherwise.
img.wp-block-cover__image-background,
video.wp-block-cover__video-background {
.wp-block-cover__image-background,
.wp-block-cover__video-background {

Copy link
Contributor Author

@ajlende ajlende Jan 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

It wasn't working for me without it, unfortunately. Without it, the specificity is the same and these styles don't get applied.

edit: for the record, the selector that I needed to override was .editor-styles-wrapper img

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the extra specificity is only needed in the editor, I could duplicate these styles in edit.scss with the extra specificity and move these out to where they were before. But, I figured it would be better to avoid duplication between edit and save wherever possible.

position: absolute;
top: 0;
left: 0;
right: 0;
bottom: 0;
margin: 0;
padding: 0;
width: 100%;
height: 100%;
max-width: none;
max-height: none;
object-fit: cover;
outline: none;
border: none;
box-shadow: none;
}
}

.wp-block-cover__video-background {
Expand Down