Inserter: Try showing a "line" between two blocks on hover to insert a new empty block #5198

youknowriad · 2018-02-22T10:24:28Z

Feedback suggests that people care about the in-between blocks inserter.
This inserter had some issues: being too distractive, some issues related to the big click area.
This PR explores an alternative showing a gray line between two blocks on hover and when you click it, you add a new empty block at that position. At that moment you'll be able to insert any other block there.

jasmussen · 2018-02-22T10:38:58Z

Just looking at the GIF, I love this, and glad to see the sibling inserter return.

I think this looks like a good implementation. It may be worth getting this one in quickly, and if it isn't obvious enough, we can fade a plus icon in as well. But given it invokes the in-canvas inserter on a newline (yay for fewer and consistent interfaces), I think this is a good first step to try.

Can the gray separator be moved down 2 pixels, so it's perfectly centered between the blocks?

Can the gray separator have left and right margins, so it's as wide as the content inside the blocks, not the contents + block padding?

With that, 👍 👍

On a personal note — I take full responsibility for removing the sibling inserter as part of the test to see if we could do without it. While I still think it was a valuable test: the results are clear that we can't do without it, given how many eyes are on Gutenberg right now it's definitely sub-optimal to test this in plugin releases, even if we are still in beta. The biggest reason being: unless people read the changelog, they have no good chance to know that it's part of a test and that the UI they relied on might return. It definitely difficult to balance the need to improve UIs and iterate fast, with the need to not alienate testers. Sorry about that, and sorry about the amount of feedback it directed negatively at you specifically. If anything it should've been directed at me.

hedgefield · 2018-02-22T10:40:23Z

Love this. Feels natural to click between blocks, I found myself doing this often when wanting to insert at a specific point.

hedgefield · 2018-02-22T10:42:28Z

And I wouldn't feel too bad about the feedback, this is still a beta, we have to be free to test things out and sometimes radically change something if it will turn out for the better. In this case, we know something we didn't know before testing all the options :)

youknowriad · 2018-02-22T10:44:59Z

Sorry about that, and sorry about the amount of feedback it directed negatively at you specifically. If anything it should've been directed at me.

Don't worry too much Joen, I don't mind receiving and replying to feedback. And I was fully behind the decision to remove the inserter for consistency.

jasmussen · 2018-02-22T10:48:33Z

Thanks folks, appreciate it. I'm personally okay with receiving a little flak for what in hindsight might have been a little haphazard. I just wish it didn't reflect onto others. Appreciate the support.

Love this. Feels natural to click between blocks, I found myself doing this often when wanting to insert at a specific point.

I agree — it's also mimicking the pattern you'd employ in a non-block editor. In Google Docs if you want to insert an image between two other images, the natural inclination is to click there and make a linebreak.

youknowriad · 2018-02-22T10:53:31Z

Updated with the tweaks. Nothing that if one adjacent block is selected, the line is a bit smaller than the border of the selected block

jasmussen · 2018-02-22T10:58:48Z

I think it looks great! Forgot to mention — can we fade in the line much faster?

My transition math isn't super great, looking at transition: opacity 0.8s linear 0.5s; does that mean fade in over a .8s period after a .5s wait? If yes, could we try instead transition: opacity 0.2s linear 0.5s;? That is, fade in over a .2s period after a .5s wait?

afercia · 2018-02-22T13:01:08Z

How this is supposed to work when using only the keyboard? I'd like to remind that any functionality should be device-independent and thus work also with keyboard only. In this discussion I see you're considering only hover and click. The button seems to always have tabIndex="-1" so it's not focusable.

At this point of the development I'd like to see features being designed since the beginning with some basic accessibility level. 🙂

youknowriad · 2018-02-22T13:06:39Z

At this point of the development I'd like to see features being designed since the beginning with some basic accessibility level.

For me this is one of the features that shouldn't be used with keyboard because it goes against tabbing between/in blocks and has alternatives with keyboard. So, with accessibility in mind, I added the tabIndex="-1" to avoid breaking accessibility of the editor. If that's wrong I'd be happy to update it with what you think is best.

afercia · 2018-02-22T13:15:03Z

Keyboard is just one type of device. There are many others. For this reason I've mentioned "device independence". Many alternative devices and software work in sort of "keyboard emulation" mode.
Actually, this is a mouse only feature. Won’t even work with touch.

I'd also like to remind the original sibling inserter (the one with the + icon) perfectly worked with keyboard. Not sure why the argumentations seems to have changed now.

youknowriad · 2018-02-22T13:18:16Z

Not sure why the argumentations seems to have changed now

That's just my argumentation, I didn't build the original sibling inserter so I can't speak for it. I'm just asking for actionable items here. What should I do? do you think I should remove the tabIndex?

hedgefield · 2018-02-22T13:29:24Z

What Andrea is trying to say is that back when the + button was in there, you could focus it with Tab and press Enter to get a new block. Not being able to focus it now is a regression in that sense. So yeah, removing the tabindex would be sufficient, if I understood correctly.

afercia · 2018-02-22T13:32:33Z

@youknowriad I'm not even sure why the original inserter with the + icon was changed :)
Anyways, I'd say the + icon worked from a functional and a11y perspective.
Reintroducing the same sibling inserter with a different look but with an a11y regression doesn't seem ideal to me. If it doesn't break anything, yes I'd like to see this inserter focusable.

brettsmason · 2018-02-22T13:41:47Z

This looks like a big improvement nice job!

My only comment from testing Gutenberg a bunch with clients/friends is a plus icon on hover/select or whatever might be nice in addition so you can directly insert a block rather than first getting the inserter to come up, thus cutting out a click.

Hope that makes sense...

youknowriad · 2018-02-22T14:12:11Z

My only comment from testing Gutenberg a bunch with clients/friends is a plus icon on hover/select or whatever might be nice in addition so you can directly insert a block rather than first getting the inserter to come up, thus cutting out a click.

We had this, and it was getting in the way of the writing flow. We need to find a balance between using Gutenberg as an editor (writing flow) and using it as a content builder (adding blocks). That's just my opinion but the current solution seems like a good middle ground approach to avoid several ways of inserting content while not making so hard.

aduth

Looks and works generally well 👍

Couple usage feedback:

At the end of content where last block is non-default, should I expect to see both the inserter button and the default block appender?
Should we have an inserter before the first block?

aduth · 2018-02-23T05:29:36Z

edit-post/assets/stylesheets/_z-index.scss

 	'.editor-block-contextual-toolbar': 2,
 	'.editor-block-switcher__menu': 2,
 	'.components-popover__close': 2,
+	'.editor-block-list__block > .editor-block-list__insertion-point':2,


Minor: Space after colon.

aduth · 2018-02-23T05:32:48Z

editor/components/block-list/insertion-point.js

-		return null;
-	}
+function BlockInsertionPoint( { showInsertionPoint, showInserter, index, layout, rootUID, ...props } ) {
+	const onClick = () => {


For a component which is rendered for every block, I think we ought to be conscious of the referential equality of functions in render reconciliation and pre-bind these into a component class.

aduth · 2018-02-23T05:35:19Z

editor/components/block-list/insertion-point.js

+function BlockInsertionPoint( { showInsertionPoint, showInserter, index, layout, rootUID, ...props } ) {
+	const onClick = () => {
+		props.insertDefaultBlock( { layout }, rootUID, index );
+		props.startTyping();


Should startTyping be an effect of insertDefaultBlock?

While we use it this way in general, I wonder if it makes sense conceptually. A third-party plugin would want to trigger insertDefaultBlock programmatically without impact on the writing flow.

youknowriad · 2018-02-23T09:36:51Z

feedback addressed, I'll defer to @jasmussen about the usability feedback from @aduth

jasmussen · 2018-02-23T09:48:24Z

At the end of content where last block is non-default, should I expect to see both the inserter button and the default block appender?

I would think not. The pattern for this iteration of the in-between/sibling inserter, is to mimic that of making a linebreak. As there already exists sort-of-a linebreak at the end of the post, this one shouldn't be necessary.

Should we have an inserter before the first block?

That would potentially be nice, it seems like? If it feels good in the branch, no reason not to.

…a new empty block

youknowriad · 2018-02-26T12:14:43Z

Feedback addressed, waiting for the tests before merging

aduth · 2018-02-26T19:28:14Z

editor/components/block-list/style.scss

@@ -396,6 +430,21 @@
 	}
 }

+.editor-block-list__layout > .editor-block-list__insertion-point {
+	position: relative;
+	margin-top: -15px;


This causes misalignment of the first block, overlapping the title and putting the text caret at the wrong position when clicking within the writing prompt:

The line also extends beyond the correct bounds of the block list / title:

Also noting that the misalignment impacts nested blocks, and should be accounted for:

(The tops of the squares should align)

aduth · 2018-02-26T19:35:42Z

This is now the default focus target for nested blocks (the bug which occurs when pressing menu). The line itself it doesn't look so great:

aduth · 2018-02-26T19:39:14Z

editor/components/block-list/layout.js

@@ -215,6 +216,7 @@ class BlockListLayout extends Component {

 		return (
 			<BlockSelectionClearer className={ classes }>
+				{ !! blockUIDs.length && <BlockInsertionPoint /> }


This needed to be passed rootUID. When trying to insert a block before the first block in a Columns block, it appears in the top-level instead.

aduth · 2018-02-26T21:01:52Z

editor/components/block-list/insertion-point.js

+			<div className="editor-block-list__insertion-point">
+				{ showInsertionPoint && <div className="editor-block-list__insertion-point-indicator" /> }
+				{ showInserter && (
+					<button


Anticipating a future bug: Arrowing up or down from a block will focus the between-inserter.

There's no good reason we don't include button in this set:

gutenberg/editor/components/writing-flow/index.js

Lines 76 to 85 in f3c1e22

getVisibleTabbables() {

return focus.tabbable

.find( this.container )

.filter( ( node ) => (

node.nodeName === 'INPUT' ||

node.nodeName === 'TEXTAREA' ||

node.contentEditable === 'true' ||

node.classList.contains( 'editor-block-list__block-edit' )

) );

}

...Actually, I'm seeing the "good reason", and it's to avoid focus moving into the block toolbar. This is very fragile, as I'd guess if someone did something like add an input (input type="button") into the toolbar, it'd break as-is.

aduth · 2018-02-26T21:47:06Z

As far as I can tell, this just doesn't work at all for nested blocks. I think it's an issue of layout not being assigned.

mcsf · 2018-02-27T18:28:35Z

This surfaced #5291.

youknowriad · 2018-02-27T18:44:26Z

It should be fixed with #5282

aduth · 2018-02-27T18:52:11Z

@youknowriad In testing #5291 while on #5282, the issue still persists. Might be something separate.

youknowriad · 2018-02-27T20:56:19Z

Right I misread the issue :) I'll take a deeper look tomorrow morning

aduth · 2018-02-27T21:21:46Z

#5291 has proposed resolution in #5296
#5198 (comment) has proposed resolution in #5295

StaggerLeee · 2018-03-02T22:45:38Z

I think you should remove block if clicked between blocks and moved away in work. Beginners tend to click around a lot, and it will result in many, really many, empty blocks.

Each click makes new block. Not each click is meant to be a block.

Have some feeling all this is unnecessary made complicated. You are haunt by reducing number of clicks. But sometimes it ends in not better things.

Clicked quickly around 14 times, and I have 14 empty blocks I do not need. Now it is 28 clicks to remove them all.

19 new (empty) blocks (after saving) for feeling around for literary about 60-120 seconds. Insane and crazy. What are you doing.

aduth · 2018-04-16T14:56:48Z

editor/components/block-list/insertion-point.js

 			),
+			showInserter: ! isTyping( state ),


I'm looking to enhance the between-inserter and am unclear the intent with this condition. Why do we only render the inserter button while not typing?

Honestly, I don't remember why I added this exactly :)

youknowriad added the [Feature] Inserter The main way to insert blocks using the + button in the editing interface label Feb 22, 2018

youknowriad self-assigned this Feb 22, 2018

youknowriad requested review from mtias and jasmussen February 22, 2018 10:24

youknowriad force-pushed the try/inbetween-inserter branch from edc4cd9 to 6054dd4 Compare February 22, 2018 10:24

youknowriad force-pushed the try/inbetween-inserter branch from d4b587e to d0d4115 Compare February 22, 2018 11:03

jasmussen mentioned this pull request Feb 22, 2018

Inserter UI disappears when i type, and it's not obvious how to get them back #5055

Closed

aduth reviewed Feb 23, 2018

View reviewed changes

youknowriad force-pushed the try/inbetween-inserter branch from f00cfc9 to 973ad7a Compare February 23, 2018 09:36

jasmussen mentioned this pull request Feb 23, 2018

adding images to existing text can only be done in 5 steps and even then the result is undersireable #2703

Closed

youknowriad force-pushed the try/inbetween-inserter branch from bb48be5 to d5f9833 Compare February 23, 2018 10:33

youknowriad added 2 commits February 26, 2018 13:12

Inserter: Try showing a "line" between two blocks on hover to insert …

c1525e4

…a new empty block

Tweak the size and the position of the in-between inserter

f3deea9

youknowriad added 4 commits February 26, 2018 13:14

Update the fade in transition speed

95da6d0

Make the sibling inserter focusable

802106c

Avoid inline functions for performance reasons

375b201

Add the insertion point at the begining of the post

c0aad6c

youknowriad force-pushed the try/inbetween-inserter branch from d5f9833 to c0aad6c Compare February 26, 2018 12:14

youknowriad merged commit 1232d37 into master Feb 26, 2018

youknowriad deleted the try/inbetween-inserter branch February 26, 2018 12:21

aduth reviewed Feb 26, 2018

View reviewed changes

youknowriad mentioned this pull request Feb 27, 2018

Inserter: Fix the in-between inserter on nested blocks #5282

Merged

mcsf mentioned this pull request Feb 27, 2018

Blocks added via editor inserter (+) inserted out of order #5291

Closed

2 tasks

aduth mentioned this pull request Feb 27, 2018

Inserter: Remove margin from initial insertion point #5295

Merged

jasmussen mentioned this pull request Mar 5, 2018

Adding a block in-between another should be easier than it is #5396

Closed

aduth reviewed Apr 16, 2018

View reviewed changes

	getVisibleTabbables() {
	return focus.tabbable
	.find( this.container )
	.filter( ( node ) => (
	node.nodeName === 'INPUT' \|\|
	node.nodeName === 'TEXTAREA' \|\|
	node.contentEditable === 'true' \|\|
	node.classList.contains( 'editor-block-list__block-edit' )
	) );
	}

Inserter: Try showing a "line" between two blocks on hover to insert a new empty block #5198

Inserter: Try showing a "line" between two blocks on hover to insert a new empty block #5198

Conversation

youknowriad commented Feb 22, 2018

jasmussen commented Feb 22, 2018

hedgefield commented Feb 22, 2018

hedgefield commented Feb 22, 2018

youknowriad commented Feb 22, 2018

jasmussen commented Feb 22, 2018

youknowriad commented Feb 22, 2018

jasmussen commented Feb 22, 2018

afercia commented Feb 22, 2018

youknowriad commented Feb 22, 2018

afercia commented Feb 22, 2018

youknowriad commented Feb 22, 2018

hedgefield commented Feb 22, 2018

afercia commented Feb 22, 2018

brettsmason commented Feb 22, 2018

youknowriad commented Feb 22, 2018 • edited Loading

aduth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad commented Feb 23, 2018

jasmussen commented Feb 23, 2018

youknowriad commented Feb 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduth commented Feb 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduth commented Feb 26, 2018

mcsf commented Feb 27, 2018

youknowriad commented Feb 27, 2018

aduth commented Feb 27, 2018

youknowriad commented Feb 27, 2018

aduth commented Feb 27, 2018

StaggerLeee commented Mar 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad commented Feb 22, 2018 •

edited

Loading

StaggerLeee commented Mar 2, 2018 •

edited

Loading