Editor: Allow title to split onto multiple lines

[aduth]

Blocked by #4824
Closes #1059
Fixes #1679

This pull request seeks to allow the editor post title to split onto multiple lines, rather than obscuring it with a long-content fade when the input loses focus.

It also introduces a new <TextareaAutosize /> component to serve as a drop-in replacement for <textarea /> in order to facilitate in this use-case.

Before	After

Implementation notes:

While the implementation does use autosize, a third-party library, note that this is already used in the editor for the HTML editing tab, so the change in editor bundle size should be negligible if anything. A follow-up task may be to consider whether the HTML tab textarea can be updated to use this component instead.

Testing instructions:

Verify in a large and small viewport size that the editor title resizes to accommodate multiple lines as needed. Ensure that existing behavior remains unaffected (notably, focusing the input for a new post, including when navigating from an existing post to a new post).

1. Navigate to the Reader
2. Start a new post using the New Post masterbar button
3. Note that the title field is auto-focused for your new post
4. Enter text into the title area
5. Note that the textarea expands to allow multiple line entry

[mtias]

I assume this is the default?

[aduth]

Nope, the rows default is 2 (reference)

[mtias]

Right, I mean the default initial value for the component.

[aduth]

Right, I mean the default initial value for the component.

Still not certain I'm following, but an empty field will default to 1 rows, and grow as necessary via autosize. Or are you suggesting we use rows="1" as a default prop for <TextareaAutosize /> ?

[mtias]

I didn't know what the prop meant just by looking at it :)

[mtias]

This is great, thanks for looking at it after I complained :)

It works well for me, and I wouldn't mind merging before #4824 if that may take longer.

[mtias]

Is there anything we could simplify markup-wise here? Some of these nested elements were needed for correctly rendering the fade effect, which we don't need anymore.

[aduth]

Is there anything we could simplify markup-wise here?

We'll likely need the relative wrapper for the loading block still, but I'll make another pass to be certain.

[mtias]

We should remove the loading block :)

[aduth]

We should remove the loading block :)

One PR at a time 😄

[aduth]

It works well for me, and I wouldn't mind merging before #4824 if that may take longer.

I want to do some last-minute performance debugging, as I was noticing an input lag in my testing. I think it may be that both the value assignment of the input field and the input's own change event (bound by autosize) are duplicating effort in resizing the field.

[aduth]

This was not previously accounting for newlines, added either by enter keypress or pasted text including multiple lines. Both cases are now accounted for in 99d89b1. Flipping this back to Needs Review.

[nylen]

I think this indentation style is clearer here:

if (
    ( isNew && ! prevProps.isNew ) ||
    ( isNew && get( prevProps.site, 'ID' ) !== get( site, 'ID' ) )
) {
    // ...
}

[aduth]

I think this indentation style is clearer here:

I go back and forth myself, but FWIW this is the documented style:

https://github.com/Automattic/wp-calypso/blob/master/docs/coding-guidelines/javascript.md#multi-line-statements

[nylen]

Works on Chrome, Safari, Firefox, Edge. Code looks good.

There are a couple of minor issues on IE11:

• Scrollbar (apparently can be eliminated with overflow: auto)
• Cursor resets to end of line on paste. (Attempting to debug this crashed IE in my VM.)

On iOS, I'm not able to paste because the text toolbar disappears immediately (also broken in master -> #4905), but other than that, the multi-line behavior seems fine.

[aduth]

@nylen : Thanks for the review. Both issues should be fixed now. Neither solutions are pretty 😄 (692ecda, cab2d16).

I'm still not entirely happy with the performance of the input with these changes. I've confirmed that the update calculations aren't being performed multiple times, but it still doesn't feel quite right. I think it's likely a combination of both the height calculation and the title saving its changes directly to the post store on each change. I'd like to move away from the latter case, but I think there are UX implications of having the option to "Save" changes as soon as characters are entered into the title field.

[nylen]

👍

[lancewillett]

OK to merge? Waiting on anything? (5 days ago.)

[aduth]

@lancewillett Works but not very happy with performance. Would be curious to get more feedback on whether it's noticeable, or at least spend more time profiling / improving performance before merging (as mentioned above, may require parallel changes to how we're tracking post title state).

[nylen]

fwiw, I didn't notice any performance problems specific to this change. The editor was pretty laggy in my IE11 VM, but that was a general issue and also true in master.

[lancewillett]

I'm marking as "In Progress" so it doesn't look like it's ready to merge by anyone.

[aduth]

Updated original comment to reflect that this incidentally fixes #1679.

[lancewillett]

Hi @aduth — checking in on this one since last update was about a month ago. Still in progress? Can we help test a branch or change to nudge it ahead?

[nylen]

Paste behavior and Safari resize behavior look good now. I couldn't think of an easy way to address the undo stack issues either, so I'll file a separate issue for that.

Also noting that when this change is merged it becomes more important to address the remaining issues with toolbar pinning (#4907) because this adds another situation where the height of editor elements above the toolbar can change.

[codebykat]

@aduth Did this fall through the cracks? Looks like it's been marked as ready to merge for awhile and the blocking PR is now resolved.

[aduth]

@codebykat : Sorta. I had planned to coordinate this merging with #5445, the other being quite a bit more risky. It doesn't need to, though might benefit from some of the changes there. In any case, I can plan to revive and merge this one this week (looks to not have any merge conflicts surprisingly, but definitely warrants a rebase all the same).

[codebykat]

Either way! I was just going through my list and it looked abandoned.

[aduth]

Pushed a rebase including a much simpler technique for blocking newlines from pasted text. Because the title is a controlled input, we can replace the newlines in the onChange handler, and this will be reflected both in the post edit state and in the next render pass.

See 1213941 for detail. I kept the preventEnterKeyPress behavior because while the onChange replace would prevent the newlines from enter keypresses, it'd either add a space with the current implementation, or even if we replaced newlines with an empty character, the autosize would already have updated the height in expectation that the newline had been added.

[gwwar]

What happens in IE when this rule isn't here?

[nylen]

An ugly Windows native scrollbar, IIRC.

[aduth]

An ugly Windows native scrollbar, IIRC.

Indeed, this one: #4826 (comment)

[gwwar]

Found a minor corner case:

1. Copy text into clipboard with new lines:

This title
Has
Three Lines

1. Paste into title
2. Select Text by double clicking
3. Paste Again

[gwwar]

Clicking into the main editor does not deselect any selected title text

[gwwar]

I left a few notes on minor corner cases, but this feels pretty solid to me. 👍

[aduth]

Found a minor corner case:

Ah, this looks to be the same underlying cause as the issue with Enter I described above.

I kept the preventEnterKeyPress behavior because while the onChange replace would prevent the newlines from enter keypresses, it'd either add a space with the current implementation, or even if we replaced newlines with an empty character, the autosize would already have updated the height in expectation that the newline had been added.

I'll plan to poke a bit to see if it's straight-forward to track when the input value could change in this way and whether adding a manual height recalculation could help (and to remove the onKeyPress callback).

[aduth]

I came up with an alternate solution in 66ad418 to correct the autosize height after replacing if value would have stayed the same. This also removes the need for the onKeyPress enter key handling.

Also rebased and squashed a bit, bumped version of autosize to latest patch version (changelog), and did another test run through all major browsers.