Writing Flow: Always add an empty at the end of the block list #3623

youknowriad · 2017-11-23T10:09:38Z

This PR

Removes the trailing inserter
Add an invisible textarea under the block list to add a new text block when needed.

codecov · 2017-11-23T10:19:12Z

Codecov Report

Merging #3623 into master will decrease coverage by 0.25%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #3623      +/-   ##
==========================================
- Coverage   36.95%   36.69%   -0.26%     
==========================================
  Files         268      268              
  Lines        6673     6649      -24     
  Branches     1202     1201       -1     
==========================================
- Hits         2466     2440      -26     
- Misses       3555     3557       +2     
  Partials      652      652

Impacted Files	Coverage Δ
editor/components/block-list/index.js	`0% <ø> (ø)`	⬆️
editor/store-defaults.js	`100% <ø> (ø)`	⬆️
editor/components/block-list/block.js	`0.74% <ø> (ø)`	⬆️
editor/reducer.js	`90% <ø> (-0.28%)`	⬇️
editor/components/block-drop-zone/index.js	`0% <ø> (ø)`
editor/selectors.js	`92.85% <ø> (-0.27%)`	⬇️
editor/edit-post/modes/visual-editor/index.js	`0% <0%> (ø)`	⬆️
editor/components/default-block-appender/index.js	`0% <0%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5ede2e...b8f960c. Read the comment docs.

jasmussen · 2017-11-23T10:24:12Z

I really, really really love this. I think it's a much better experience without all that trailing chrome, and it's a much clearer way to insert content after non-text blocks. Here's a brief GIF:

I'd love @mtias and @karmatosed's opinion (but I'm crossing my fingers they'll like this as much as I do), and we want to make sure that empty textarea is announced correctly to screen readers (cc: @afercia for a quick test perhaps). Finally it looks like we may in fact need #3485 after all (despite my comment here).

jasmussen · 2017-11-23T10:58:41Z

Small update — we'll still want the placeholder to look and function like it used to on an empty post:

That is — with the "Write your story" prompt, and the border on hover.

The chief reason why the border and text prompt shown at the end of the blocklist on a populated page should be less visible, only indicated by the hover ibeam cursor, is that there isn't actually a block there. I know that's the same at the start of a post, but we do need a prompt and some canvas indication there.

youknowriad · 2017-11-23T11:15:19Z

Updated with the visible placeholder when post is empty

afercia · 2017-11-23T12:27:21Z

Tested a bit with keyboard only and also with screen readers. Seems the block is announced correctly, as it just uses the default aria-label Add text or type / to insert content. Improvements to the Editable blocks (role and aria-label) are discussed on other issues.

However, there's a big issue 🙂 Right now, it's a keyboard trap. Whatever I do, using only the keyboard with no screen reader running or also with a screen reader, when I tab away from the lazt block I always get a new paragraph and there's no escape. This breaks any attempt to navigate the page using the keyboard.

At each Tab press, a new paragraph gets added:

Also, the paragraphs get saved in the content that soon becomes full of empty paragraphs:

<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->

youknowriad · 2017-11-23T12:30:51Z

@afercia What solution do you propose to this?

The only option I can think of is making this "untabbable" when the post is not empty which make it useless using keyboard but I think it's more targeted towards mouse users?

youknowriad · 2017-11-23T12:32:41Z

Edit: Actually I may have a better option, transform this placeholder to a "button" only actionable on click/enter

jasmussen · 2017-11-23T12:41:37Z

Forgive me for asking a possibly silly question: why does tabbing away from the placeholder textarea create a block? Shouldn't it just focus whatever element comes next in the flow after the placeholder?

youknowriad · 2017-11-23T12:42:06Z

updated, there's no tab trap anymore but I may be missing some ARIA labels

jasmussen · 2017-11-23T12:47:58Z

If a button is kosher to use and that works well, then cool — but a followup question: is it then kosher to apply cursor: text; to that button? It's visually important to mouse users to indicate what happens when you click there, and a text/ibeam cursor is the best way to do so.

afercia · 2017-11-23T13:05:10Z

It's visually important to mouse users to indicate what happens when you click there,

I also think from an usability perspective, there should be some indication when hovering that area. A possible problem is it kinds of overlap the "drop zone"?

afercia · 2017-11-23T13:21:26Z

Tested with latest commit, there's no more a keyboard trap and that's good 🙂

However, when tabbing from the title to the first "Write your story" there's no indication of focus, so there's a tab stop but I don't know where I am.

Instead, after I add something and tab again, all the following buttons have a light gray border to indicate focus:

This is maybe too light and should be improved a bit but I guess it's outside of the scope of this PR. I'd second Joen: maybe the cursor should be a pointer but then maybe there should be some additional visual indication on hover.

youknowriad · 2017-11-23T13:30:35Z

maybe the cursor should be a pointer but then maybe there should be some additional visual indication on hover.

I think for mouse users the "text" cursor is a better visual indication than the "hover" style. The "hover" style on an empty area is distracting IMO.

However, when tabbing from the title to the first "Write your story" there's no indication of focus, so there's a tab stop but I don't know where I am.

This is fixed

jasmussen · 2017-11-24T07:58:11Z

Really nice work Riad. Here's a recording:

I think we're 99% there.

There's just one issue now — arrow down from the title to start typing broke, and if you tab from the title, the button has focus but it still needs to be "clicked". Anything we can do here?

jasmussen · 2017-11-24T08:02:19Z

Possibly related to the "keep typing" thing: #3558

youknowriad · 2017-11-24T08:17:22Z

That's related to the fix for the tab trap, if you want to keep typing it should transform to a text block when it receives the focus.

A middle ground would be to do so only for empty posts.

What do you think?

jasmussen · 2017-11-24T08:23:06Z

A middle ground would be to do so only for empty posts.

This sounds good to me.

One feature that works in master is that if the image is the last block in the list, and you select it and press Enter, then you can start typing again. Not very discoverable which is why I really like this branch and approach. However can we retain the feature? That is, if you press enter on the last image in a blocklist, you can type?

gziolo · 2017-11-24T08:26:17Z

editor/components/default-block-appender/style.scss

+		box-shadow: none;
+		display: block;
+		width: 100%;
+		height: $text-editor-font-size * 6; // 1.5 times the height of an empty paragraph


It's worth considering to introduce new scss variable $empty-paragraph-height to avoid those comments.

gziolo · 2017-11-24T08:33:18Z

editor/components/default-block-appender/index.js

+	}
+
+	appendDefaultBlock() {
+		const newBlock = createBlock( getDefaultBlockName() );


Would it make sense to update createBlock definition to simplify this call to:

export function createBlock( name = getDefaultBlockName(), blockAttributes = {} ) { ... } this.props.onInsertBlock( createBlock() );

I don't know :)

gziolo · 2017-11-24T08:36:54Z

editor/components/default-block-appender/index.js

+			<div className={ className }>
+				<BlockDropZone />
+				<button onClick={ this.appendDefaultBlock }>
+					{ count === 0 && __( 'Write your story' ) }


Nice and simple 👍

gziolo · 2017-11-24T08:39:16Z

editor/components/default-block-appender/index.js

+	}
+}
+
+export default connect(


it would be nice to have 3 unit tests:

count > 0 (snapshot test)

count = 1 check if there is class is-visible-placeholder and Write your story text

spy on insertBlock

I can add them, just ping me :)

…ceholder

gziolo

I reviewed code part and it looks great. This change also removes some more complex logic which I'm always happy about. I will add unit tests for the newly introduced placeholder component. Don't wait for me, this is too nice change to postpone it :D

mtias · 2017-11-24T12:36:13Z

Sorry for not replying here earlier, but I think this is a bit rough and would need some further work and polish to be better than what we had, in my opinion.

Empty paragraphs even if you don't write anything.

If you click on the bottom of the post just to deselect something, you end up with an empty paragraph that you need to remove. This is not a great user experience.

If you click the bottom area, and then click something else, no paragraph should be left.

Jumpy Container

When you click on the empty area, there is a foucs area that is larger than what the paragraph would end up being, so the visual border jumps after click:

Preserving the Plus

I think Medium's screenshot in the linked issue strikes a better balance. At the moment it's not clear you can add other stuff other than text. The / command is a power user feature, and there is no indication it works at all until you see an empty paragraph's placeholder.

I strongly suggest retaining the plus, maybe show it when hovering the bottom area, and remove it once you start typing.

Treat it as if `isEditing` Continues

The other thing that bothers me is that the UI for the bottom block appears as soon as you click. That defeats the effect seeked, in my opinion. Clicking on the bottom area should just place the cursor there without the block UI, meaning it should engage isEditing mode, as you are not selecting a block but trying to continue writing.

jasmussen · 2017-11-24T14:49:18Z

Perhaps this PR was merged a bit quickly, but nothing's so bad that we can't fix it for 1.8 with a little polish here and there:

the jumpy container is my fault — I made it bigger, I shouldn't have, easy fix, just undo 29768ab
We should also get the arrow-key navigation from the title to the first placeholder block working, I believe that's a regression.
"If you click the bottom area, and then click something else, no paragraph should be left" is a good suggestion.

Those seem like good ones to address first.

Treat it as if isEditing Continues and Preserving the plus could potentially be solved together, or at least dictate how they should be implemented.

That is to say, right now the behavior of the trailing placeholder — the one at the end of a non empty post, not the first one on an empty post — is to create a new block when clicked/invoked. Given the model here is that of Medium, which I agree works very well, perhaps this trailing placeholder can be... let's call it "Schrödingers placeholder" — it exists, and doesn't exist, until you take an action on it.

Imagine you have a post with just an image in it. It could then work like this:

Hover below the image block, see the "text" cursor.
Click this area, the caret is now in this spot, and there's an inserter shortcut on the left, but no paragraph block has yet been created
3a. Start typing — it's now a paragraph block, and you're typing so no borders or chrome are showing.
3b. Click away — no paragraph block was created.
3c. Click the inserter — you get the inserter.

Mockups. First you hover near the end:

Then you click the "textfield", but no paragraph is created yet:

If you start typing, that's when the paragraph is created:

Edit: Updated the mockups above to have an image instead of text.

Thoughts? @mtias @gziolo @youknowriad @karmatosed

youknowriad · 2017-11-24T15:03:38Z

We should also get the arrow-key navigation from the title to the first placeholder block working, I believe that's a regression.

This was working and got broken in one of the last changes, I'll fix it.

"If you click the bottom area, and then click something else, no paragraph should be left" is a good suggestion.

That's not an easy thing to do, because when we click it's transformed to a text block and we'd have to remove empty text blocks at the end or something and it's not easy to know when to do this exactly.

Click this area, the caret is now in this spot, and there's an inserter shortcut on the left, but no paragraph block has yet been created
3a. Start typing — it's now a paragraph block, and you're typing so no borders or chrome are showing.
3b. Click away — no paragraph block was created.
3c. Click the inserter — you get the inserter.

Same, this is doable but may take some time because typing in a input should transform to a text block and keep the cursor at the right position.

Maybe we should revert this PR and try to address those

mtias · 2017-11-24T15:07:13Z

That's not an easy thing to do, because when we click it's transformed to a text block and we'd have to remove empty text blocks at the end or something and it's not easy to know when to do this exactly.

I was thinking it would be a "mode" of the paragraph block that initializes as isEditing on click, and instead of block mover shows an inserter. On blur, if this special paragraph block is empty, we remove it.

Agree on reverting if we need more time to figure out implementation.

aduth · 2018-02-27T01:38:55Z

editor/components/default-block-appender/index.js

+						readOnly
+						onFocus={ this.appendDefaultBlock }
+						onClick={ this.appendDefaultBlock }
+						onKeyDown={ this.appendDefaultBlock }


In what case could a keydown event occur without the block already being focused? (And therefore triggering the identical focus event handler)

Good catch, probably a useless event handler.

youknowriad self-assigned this Nov 23, 2017

youknowriad requested a review from jasmussen November 23, 2017 10:09

youknowriad force-pushed the update/text-placeholdeer-always-visible branch from 021ffe1 to 93a16f9 Compare November 23, 2017 13:29

youknowriad and others added 5 commits November 23, 2017 14:37

Writing Flow: Always add an empty at the end of the block list

33fafb4

Increase appender height just a little bit.

29768ab

Tests: Fix integration tests after inserter removal

11a1eca

Cleanup: Remove the quick inserter code and state deps

7265186

BLock Appender: Show the placeholder when the post is empty

9b619a7

youknowriad force-pushed the update/text-placeholdeer-always-visible branch from 93a16f9 to 441cfc4 Compare November 23, 2017 13:38

gziolo reviewed Nov 24, 2017

View reviewed changes

Block Appender: Use a button instead of input for the "invisible" pla…

b8f960c

…ceholder

youknowriad force-pushed the update/text-placeholdeer-always-visible branch from 441cfc4 to b8f960c Compare November 24, 2017 08:50

gziolo approved these changes Nov 24, 2017

View reviewed changes

youknowriad merged commit 42f9b1a into master Nov 24, 2017

youknowriad deleted the update/text-placeholdeer-always-visible branch November 24, 2017 09:00

gziolo mentioned this pull request Nov 24, 2017

Editor: Add tests for DefaultBlockAppender #3638

Merged

3 tasks

youknowriad mentioned this pull request Nov 24, 2017

Revert/empty placeholder #3650

Merged

jasmussen mentioned this pull request Dec 7, 2017

Consider removing the trailing inserter in favor of a clickable area #3078

Closed

mcsf mentioned this pull request Dec 13, 2017

Writing Flow: 'Down' key at bottom creates new paragraph #3973

Merged

3 tasks

aduth reviewed Feb 27, 2018

View reviewed changes

aduth mentioned this pull request Aug 8, 2018

Default Block Appender: Remove redundant event handlers #8714

Merged

earnjam mentioned this pull request Aug 17, 2018

Revisit the "Write your story" prompt #9076

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Writing Flow: Always add an empty at the end of the block list #3623

Writing Flow: Always add an empty at the end of the block list #3623

youknowriad commented Nov 23, 2017 •

edited

codecov bot commented Nov 23, 2017 •

edited

jasmussen commented Nov 23, 2017

jasmussen commented Nov 23, 2017

youknowriad commented Nov 23, 2017

afercia commented Nov 23, 2017

youknowriad commented Nov 23, 2017

youknowriad commented Nov 23, 2017

jasmussen commented Nov 23, 2017

youknowriad commented Nov 23, 2017

jasmussen commented Nov 23, 2017

afercia commented Nov 23, 2017

afercia commented Nov 23, 2017

youknowriad commented Nov 23, 2017

jasmussen commented Nov 24, 2017

jasmussen commented Nov 24, 2017

youknowriad commented Nov 24, 2017

jasmussen commented Nov 24, 2017

gziolo Nov 24, 2017

gziolo Nov 24, 2017

youknowriad Nov 24, 2017

gziolo Nov 24, 2017

gziolo Nov 24, 2017

gziolo left a comment

mtias commented Nov 24, 2017 •

edited

jasmussen commented Nov 24, 2017 •

edited

youknowriad commented Nov 24, 2017

mtias commented Nov 24, 2017

aduth Feb 27, 2018

youknowriad Feb 27, 2018

Writing Flow: Always add an empty at the end of the block list #3623

Writing Flow: Always add an empty at the end of the block list #3623

Conversation

youknowriad commented Nov 23, 2017 • edited

codecov bot commented Nov 23, 2017 • edited

Codecov Report

jasmussen commented Nov 23, 2017

jasmussen commented Nov 23, 2017

youknowriad commented Nov 23, 2017

afercia commented Nov 23, 2017

youknowriad commented Nov 23, 2017

youknowriad commented Nov 23, 2017

jasmussen commented Nov 23, 2017

youknowriad commented Nov 23, 2017

jasmussen commented Nov 23, 2017

afercia commented Nov 23, 2017

afercia commented Nov 23, 2017

youknowriad commented Nov 23, 2017

jasmussen commented Nov 24, 2017

jasmussen commented Nov 24, 2017

youknowriad commented Nov 24, 2017

jasmussen commented Nov 24, 2017

gziolo Nov 24, 2017

Choose a reason for hiding this comment

gziolo Nov 24, 2017

Choose a reason for hiding this comment

youknowriad Nov 24, 2017

Choose a reason for hiding this comment

gziolo Nov 24, 2017

Choose a reason for hiding this comment

gziolo Nov 24, 2017

Choose a reason for hiding this comment

gziolo left a comment

Choose a reason for hiding this comment

mtias commented Nov 24, 2017 • edited

Empty paragraphs even if you don't write anything.

Jumpy Container

Preserving the Plus

Treat it as if isEditing Continues

jasmussen commented Nov 24, 2017 • edited

youknowriad commented Nov 24, 2017

mtias commented Nov 24, 2017

aduth Feb 27, 2018

Choose a reason for hiding this comment

youknowriad Feb 27, 2018

Choose a reason for hiding this comment

youknowriad commented Nov 23, 2017 •

edited

codecov bot commented Nov 23, 2017 •

edited

mtias commented Nov 24, 2017 •

edited

Treat it as if `isEditing` Continues

jasmussen commented Nov 24, 2017 •

edited