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

Try: Refactor contextual toolbar to work better with floats #11357

Merged
merged 5 commits into from
Nov 9, 2018
Merged
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
5 changes: 4 additions & 1 deletion packages/block-library/src/classic/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,12 @@ div[data-type="core/freeform"] .editor-block-contextual-toolbar + div {
// So we move it to the right, and make room for it.
@include break-small() {
.editor-block-list__block[data-type="core/freeform"] {
.editor-block-switcher__no-switcher-icon {
display: none;
}
.editor-block-contextual-toolbar {
float: right;
margin-right: -$block-side-ui-clearance - $border-width;
margin-right: $icon-button-size - $block-padding + $border-width;
transform: translateY(-#{ $block-padding - $border-width });
top: $block-padding;

Expand Down
11 changes: 6 additions & 5 deletions packages/block-library/src/columns/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@
}

@include break-small() {
> .editor-block-contextual-toolbar {
> .editor-block-list__block-edit > .editor-block-contextual-toolbar {
top: $block-toolbar-height - $border-width;
transform: translateY(-$block-toolbar-height - $border-width);
margin-left: -$block-padding - $block-padding - $border-width;
margin-left: -$grid-size-large - $border-width;
}

> .editor-block-list__block-edit::before {
Expand All @@ -82,8 +82,9 @@
left: 0;
}

> .editor-block-list__breadcrumb {
margin-right: -$block-padding - $border-width;
> .editor-block-list__block-edit > .editor-block-list__breadcrumb {
top: 0;
right: 0;
}
}

Expand Down Expand Up @@ -133,6 +134,6 @@
pointer-events: none;
}

:not(.components-disabled) > .wp-block-columns > .editor-inner-blocks > .editor-block-list__layout > [data-type="core/column"] > .editor-block-list__block-edit > .editor-inner-blocks {
:not(.components-disabled) > .wp-block-columns > .editor-inner-blocks > .editor-block-list__layout > [data-type="core/column"] > .editor-block-list__block-edit .editor-inner-blocks {
pointer-events: all;
}
20 changes: 14 additions & 6 deletions packages/edit-post/src/components/visual-editor/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@

// Center the block toolbar on wide and full-wide blocks.
// Use specific selector to not affect nested block toolbars.
&[data-align="wide"] > .editor-block-contextual-toolbar,
&[data-align="full"] > .editor-block-contextual-toolbar {
width: calc(100% + #{ $block-side-ui-width * 2 + $block-padding * 2 + $border-width * 2 }); // Matches the negative margins applied to parent blocks.
&[data-align="wide"] > .editor-block-list__block-edit > .editor-block-contextual-toolbar,
&[data-align="full"] > .editor-block-list__block-edit > .editor-block-contextual-toolbar {
width: calc(100% + #{ $block-padding * 2 + $border-width * 2 }); // Matches the negative margins applied to parent blocks.
height: 0; // This collapses the container to an invisible element without margin.
text-align: center;

Expand All @@ -42,9 +42,11 @@
}
}

// The centering math changes when a fullwide image doesn't have block padding.
&[data-align="full"] > .editor-block-contextual-toolbar {
width: calc(100% + #{ $block-side-ui-width * 2 + $block-padding * 2 }); // Matches the negative margins applied to non-parent blocks, except for borders which are gone in fullwide.
// The centering math is simpler for a fullwide block, which doesn't have any block padding.
&[data-align="full"] > .editor-block-list__block-edit > .editor-block-contextual-toolbar {
width: 100%;
margin-left: 0;
margin-right: 0;

.editor-block-toolbar {
max-width: $content-width - $border-width - $border-width;
Expand Down Expand Up @@ -84,6 +86,12 @@
}

.edit-post-visual-editor {
// If the first block is floated, it needs top margin, unlike the rule in line 69.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a little awkward to have a special rule like this. But it is due to the negative margins that every block, except floats, have. It would be nice to refactor this rule to be unnecessary in the future. Whether this is through removing all the negative margins by making them unnecessary (unlikely) or whether it's through changing even floats so they behave the same as other blocks.

If the title was an actual block, then it would be a non issue.

.editor-block-list__layout > .editor-block-list__block[data-align="left"]:first-child,
.editor-block-list__layout > .editor-block-list__block[data-align="right"]:first-child {
margin-top: $block-padding + $block-spacing + $border-width + $border-width + $block-padding;
}

.editor-default-block-appender {
// Default to centered and content-width, like blocks
margin-left: auto;
Expand Down
67 changes: 34 additions & 33 deletions packages/editor/src/components/block-list/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -515,45 +515,46 @@ export class BlockListBlock extends Component {
onDragEnd={ this.onDragEnd }
/>
) }
{ shouldShowBreadcrumb && (
<BlockBreadcrumb
clientId={ clientId }
isHidden={ ! ( isHovered || isSelected ) || hoverArea !== 'left' }
/>
) }
{ shouldShowContextualToolbar && <BlockContextualToolbar /> }
{ isFirstMultiSelected && (
<BlockMultiControls rootClientId={ rootClientId } />
) }
<IgnoreNestedEvents
ref={ this.bindBlockNode }
onDragStart={ this.preventDrag }
onMouseDown={ this.onPointerDown }
className="editor-block-list__block-edit"
data-block={ clientId }
>
<BlockCrashBoundary onError={ this.onBlockError }>
{ isValid && blockEdit }
{ isValid && mode === 'html' && (
<BlockHtml clientId={ clientId } />
) }
{ ! isValid && [
<BlockInvalidWarning
key="invalid-warning"
block={ block }
/>,
<div key="invalid-preview">
{ getSaveElement( blockType, block.attributes ) }
</div>,
] }
</BlockCrashBoundary>
{ shouldShowMobileToolbar && (
<BlockMobileToolbar
<div className="editor-block-list__block-edit">
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain a bit why the changes here. These are the most impacting changes I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I got it from the description of the PR

{ shouldShowBreadcrumb && (
<BlockBreadcrumb
clientId={ clientId }
isHidden={ ! ( isHovered || isSelected ) || hoverArea !== 'left' }
/>
) }
{ !! error && <BlockCrashWarning /> }
</IgnoreNestedEvents>
{ shouldShowContextualToolbar && <BlockContextualToolbar /> }
<IgnoreNestedEvents
ref={ this.bindBlockNode }
onDragStart={ this.preventDrag }
onMouseDown={ this.onPointerDown }
data-block={ clientId }
>
<BlockCrashBoundary onError={ this.onBlockError }>
{ isValid && blockEdit }
{ isValid && mode === 'html' && (
<BlockHtml clientId={ clientId } />
) }
{ ! isValid && [
<BlockInvalidWarning
key="invalid-warning"
block={ block }
/>,
<div key="invalid-preview">
{ getSaveElement( blockType, block.attributes ) }
</div>,
] }
</BlockCrashBoundary>
{ shouldShowMobileToolbar && (
<BlockMobileToolbar
clientId={ clientId }
/>
) }
{ !! error && <BlockCrashWarning /> }
</IgnoreNestedEvents>
</div>
{ showEmptyBlockSideInserter && (
<Fragment>
<div className="editor-block-list__side-inserter">
Expand Down
Loading