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

Sibling inserter: Fix the inserter position for floated blocks #4146

Merged
merged 1 commit into from
Jan 10, 2018
Merged

Sibling inserter: Fix the inserter position for floated blocks #4146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions editor/components/block-list/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import BlockHtml from './block-html';
import BlockContextualToolbar from './block-contextual-toolbar';
import BlockMultiControls from './multi-controls';
import BlockMobileToolbar from './block-mobile-toolbar';
import BlockListSiblingInserter from './sibling-inserter';
import {
clearSelectedBlock,
editPost,
Expand Down Expand Up @@ -439,6 +440,7 @@ export class BlockListBlock extends Component {
{ showUI && <BlockMobileToolbar uid={ block.uid } /> }
</div>
{ !! error && <BlockCrashWarning /> }
<BlockListSiblingInserter uid={ block.uid } />
Copy link
Member

Choose a reason for hiding this comment

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

I am not certain moving sibling inserter inside block-list will not have the undesirable side effect of making sibling inserter unusable for locked blocks cc: @gziolo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean by "locked 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.

I guess you mean "template locking". In which case, this change doesn't change the current behavior which is: The Inserter component does not show anything if the template is locked. This component is included in the BlockListSiblingInserter component.

Copy link
Member

@jorgefilipecosta jorgefilipecosta Dec 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

In PR #3493 BlockListBlock was made extensible to allow the creation of Frozen/locked blocks. Blocks that are temporarily not editable because of co-editing for example. It looks like the hook allows us to "change" BlockListBlock, so in places that use/or we have plans to use this hook, we may be disabling sibling inserter.

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 point! If that means the hook should be "smarter" or the hook should target BlockEdit instead?

Copy link
Member

Choose a reason for hiding this comment

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

In accordance with the conversation on PR #3493 adding the hook to BlockEdit does not fit the needs, because it would not be possible to lock the toolbar or the block transform mechanism. So we can make the hook smarter or we can try to add a component that wraps BlockListBlock and the sibling inserter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gziolo any thoughts on this?

Copy link
Member

Choose a reason for hiding this comment

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

@jorgefilipecosta thanks for raising this issue. Yes, it might be a bit inconvenient, but I'm sure we can find workarounds :) Don't treat it as a blocker as we can figure out solutions later.

So we can make the hook smarter or we can try to add a component that wraps BlockListBlock and the sibling inserter.

It seems like a wrapping div would work perfectly fine. I like the idea. I didn't think about it too much though :)

</div>
);
/* eslint-enable jsx-a11y/no-static-element-interactions, jsx-a11y/onclick-has-role, jsx-a11y/click-events-have-key-events */
Expand Down
12 changes: 4 additions & 8 deletions editor/components/block-list/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { connect } from 'react-redux';
import {
findLast,
flatMap,
map,
invert,
isEqual,
mapValues,
Expand Down Expand Up @@ -211,20 +211,16 @@ class BlockList extends Component {
return (
<div>
{ !! blocks.length && <BlockListSiblingInserter /> }
{ flatMap( blocks, ( uid ) => [
{ map( blocks, ( uid ) => (
<BlockListBlock
key={ 'block-' + uid }
uid={ uid }
blockRef={ this.setBlockRef }
onSelectionStart={ this.onSelectionStart }
onShiftSelection={ this.onShiftSelection }
showContextualToolbar={ showContextualToolbar }
/>,
<BlockListSiblingInserter
key={ 'sibling-inserter-' + uid }
uid={ uid }
/>,
] ) }
/>
) ) }
</div>
);
}
Expand Down
27 changes: 19 additions & 8 deletions editor/components/block-list/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,15 +391,13 @@
.editor-block-list__sibling-inserter {
z-index: z-index( '.editor-block-list__sibling-inserter' );
position: relative;
max-width: $visual-editor-max-width + ( 2 * $block-mover-padding-visible ) - ( 2 * $block-padding );
margin: 0 auto;

@include break-small {
max-width: $visual-editor-max-width - ( 2 * $block-padding );
}

&:not( [data-insert-index="0"] ) {
top: #{ -1 * ( $block-spacing / 2 ) };
&[data-insert-index="0"] {
max-width: $visual-editor-max-width + ( 2 * $block-mover-padding-visible ) - ( 2 * $block-padding );
margin: 0 auto;
@include break-small {
max-width: $visual-editor-max-width - ( 2 * $block-padding );
}
}

&:before {
Expand Down Expand Up @@ -454,6 +452,19 @@
}
}

.editor-block-list__block > .editor-block-list__sibling-inserter {
position: absolute;
bottom: 0;
top: auto;
left: 0;
right: 0;

@include break-small {
left: $block-mover-padding-visible;
right: $block-mover-padding-visible;
}
}

$sticky-bottom-offset: 20px;

.editor-block-contextual-toolbar {
Expand Down