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.

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 floats in a full-bleed world #799

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions blocks/library/embed/style.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
.blocks-embed {
margin: 0;
clear: both; // necessary because we use responsive trickery to set width/height, and therefore the video doesn't intrinsically clear floats like an img does
}

.blocks-embed .iframe-overlay {
Expand Down
8 changes: 8 additions & 0 deletions blocks/library/image/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,19 @@
&[data-align="left"] {
float: left;
margin-right: $block-padding;

@include break-medium() {
margin-left: calc( 50% - #{ $visual-editor-max-width / 2 } );
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Aside: Do we expect block authors to implement this CSS styles (which are not that easy) in each floated block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good aside. And no, I've been thinking of it this way:

  • Full-width can be implemented in many different ways, we've chosen one of many ways
  • Theme authors should probably opt-in to the feature, probably something like add_theme_support( 'full-width-images' );
  • When the theme has opted in, images simply get align-full-width as a CSS class. Or something in that vein.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm asking about block authors too. I mean, should we move these styles outside the image block stylesheet and provide a way to reuse it across different blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, good point. Taking a look.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we move the visual styles out as part of this PR, or as part of for example #729? Tackling that one seems like a good successor to this one, as video should also have full width as an option.

}

&[data-align="right"] {
float: right;
margin-left: $block-padding;

@include break-medium() {
margin-right: calc( 50% - #{ $visual-editor-max-width / 2 } );
}
}

&[data-align="wide"] {
Expand Down
14 changes: 5 additions & 9 deletions editor/modes/visual-editor/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,14 @@
}
}

$sticky-bottom-offset: 20px;
.editor-visual-editor__block-controls {
@include animate_fade;
display: flex;
position: sticky;
z-index: z-index( '.editor-visual-editor__block-controls' );
margin-top: -$block-controls-height - $item-spacing;
margin-bottom: $item-spacing + 20px; // 20px is the offset from the bottom of the selected block where it stops sticking
margin-bottom: $item-spacing + $sticky-bottom-offset;
height: $block-controls-height;

top: $header-height + $admin-bar-height-big + $item-spacing;
Expand All @@ -88,14 +89,9 @@
}

.editor-visual-editor__block-controls + div {
margin-top: -20px;

// prevent collapsing margins between the block and the toolbar
&:before {
content: "";
display: table;
clear: both;
}
// prevent collapsing margins between block and toolbar, matches the 20px bottom offset
margin-top: -$sticky-bottom-offset - 1px;
padding-top: 1px;
}

.editor-visual-editor__block-controls .components-toolbar {
Expand Down