FSE: Fix description block jumping around in the editor #35065

noahtallen · 2019-08-01T20:25:46Z

This fixes an issue where the text would jump around when selecting the description textbox in the post editor.

No Jumping:

This will also give us more flexibility in the future to expand what users can do with this block. For example, we can disable multiline input and allow people to do bold/italics and stuff like that. For now, I have not included that support since it will mean changing the rendering logic in php, which is beyond the scope of this PR.

Changes proposed in this Pull Request

Use RichText instead of PlainText for the block. This works because PlainText uses an autoresizer library under the hood. I think that library causes the jumping around. In this case, RichText does not use that library.

Testing instructions

Pull the code and run it in your local FSE environment
Navigate to the header template part editor
Click into the text of the site description block. Previously, this would have caused the text to jump around. (See FSE: Site Description block "jumps" when focused #34886.)
Verify that you see no jumping around even after making changes to the text, selecting other blocks, and selecting in and out of the description block.
Verify that making a change to the text in the site description block and updating it still causes the front end to update correctly.

Fixes #34886

- This fixes an issue where the text would jump around when selecting the description textbox in the post editor. - This will also give us more flexibility in the future to expand what users can do with this block.

matticbot · 2019-08-01T20:25:47Z

Test live: https://calypso.live/?branch=fix/site-description-jump

matticbot · 2019-08-01T20:33:43Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

gwwar · 2019-08-01T21:43:52Z

...full-site-editing/full-site-editing-plugin/full-site-editing/blocks/site-description/edit.js

 				className={ className }
 				value={ option }
+				tagName="p"
+				formattingControls={ [] }


Ah interesting, does this prevent keyboard commands for applying things like bold styling?

Looks like that's not the case testing on wpcom. Let's stick with the PlainText component and update editor styling to better match the block selected state (with selection) vs preview state. Let us know if this is tricky to find, we'd be happy to help.

The description is going to be plain text, so any html content will be escaped like:

So formattingControls={ [] } is supposed to remove formatting controls from the RichText block - is that a bug where it doesn't work on wpcom? I used this specifically to get rid of the escaped content issue :(

I'm testing on gutenberg master so maybe it's a new feature?

I'm testing on gutenberg master so maybe it's a new feature?

Can maybe verify by checking out back to v6.1.1 though I wouldn't assume so?

gwwar · 2019-08-01T22:03:07Z

This works because PlainText uses an autoresizer library under the hood.

Curious what the original use case for that is, re-size via dragging the text area? I wonder if we can pass a setting for single line support only to the component? This would also fix #34419

noahtallen · 2019-08-01T23:21:12Z

update editor styling to better match the block selected state

@gwwar I did initially try to fix this with just theme styling. The jumping around after initial selection is caused by an extra bit of margin that gets applied in the focused state in the modern business theme. However, there is still a non theme styling issue after that. As you can see here: #34886 (comment) the height is manually set in such a way that we can't easily override it via the theme's editor stylesheet. Thankfully, the margin issue doesn't happen when we use RichText, which is nice. :)

One fix for that is to specify style={ maxHeight: 19px } on the PlainText component here, as that gets passed into the autoresizer library. I didn't think that would work for other themes which might want bigger text, so I avoided it.

Per textarea settings, you can use maxRows, but that doesn't avoid newlines and such. And doesn't fix the jumping around :)

Personally, I like that RichText allows us to support a lot more than PlainText if we want to in the future (i.e. why not allow a user to bold a word in their description?) as long as we can avoid the escaping issues until we fix up everywhere that consumes it. Rich text also provides support out of the box for single line behavior.

I can try to see what causes the library to give it a higher hight in the beginning though since RichText won't work for us on wpcom yet!

apeatling · 2019-08-01T23:26:32Z

Personally, I like that RichText allows us to support a lot more than PlainText if we want to in the future (i.e. why not allow a user to bold a word in their description?) as long as we can avoid the escaping issues until we fix up everywhere that consumes it. Rich text also provides support out of the box for single line behavior.

I'd would expect us to support font sizes, faces, variations, and even colors in the future. Users will definitely want to change the text site, and font (beyond any global style settings we give them).

gwwar · 2019-08-01T23:28:13Z

I'd would expect us to support font sizes, faces, variations, and even colors in the future.

@apeatling Should we ditch the Site Description block then and put in a paragraph block?

gwwar · 2019-08-01T23:31:16Z

I can try to see what causes the library to give it a higher hight in the beginning though since RichText won't work for us on wpcom yet!

It's not that RichText can't work, we also need to account for our own customizations, like adding underline/justify. https://github.com/Automattic/wp-calypso/tree/master/apps/wpcom-block-editor/src/common

If we're okay with the RichText component, I'm sure we can make it work with enough debugging.

noahtallen · 2019-08-01T23:33:25Z

Yeah, I wonder if we could pass through the prop to disable customizations for now. I didn't realize we had a wpcom override 🙃

apeatling · 2019-08-01T23:34:34Z

I don't want to hold up anything here because of possible things we might do in the future. So I am fine with doing whatever is easiest to fix this right now.

gwwar · 2019-08-01T23:36:35Z

If you have time @Copons, appreciated if you can help troubleshoot.

@noahtallen see #28696 for context on why we have an override for RichText.

noahtallen · 2019-08-02T00:12:56Z

Darn. I can't figure out why it ignores the prop on wpcom. We aren't modifying rich text, we're just adding more format options. I was really hoping we could stick with this since it seems like we'll have to modify all of the theme styles too.

Copons · 2019-08-02T13:14:07Z

This appears to be caused by a bottom margin added by the theme:

https://github.com/Automattic/themes/blob/0afe9f26f9b91d3710fa5a9d32ac5b1d9a8210ce/modern-business/sass/site/header/_site-header.scss#L117

See what happens when I toggle it in the dev tools:

noahtallen · 2019-08-02T16:13:46Z

@Copons you're definitely right, that's the first thing I tried to fix ;) Unfortunately, even when I fix the margin issue, there is another jump which only happens when clicking into the box for the first time. Check this out:

With the margin fixed, it doesn't jump around after the first time, but the height is getting set manually and specifying the correct height in CSS doesn't override the inline style. I assume (though I need to do more testing) that the inline style for the height is set by the autoresizer library, which is why I tried the RichText solution

Copons · 2019-08-02T16:40:52Z

you're definitely right, that's the first thing I tried to fix ;)

Ha! Shame on me for scanning the comments a bit too quickly! 🤦‍♂

What about adjusting both the margin the line height?
It seems that after removing the bottom margin, the text does not start vertically centered (container height: 28px, line height: 19px), but a bit towards the top.
When we focus the block, all kinds of magic triggers: the text repositions and we see the jump.

On the other hand, with this I can't see any jumps anymore (but my eyes are tired, so I might be wrong 🤔):

.site-description, 
.wp-block-a8c-site-description,
.wp-block-a8c-site-description:focus {
	color: $color__text-main;
	font-size: $font__size-xs;
	font-weight: 300;
-	margin: 0 0 calc(0.5 * #{$size__spacing-unit});
+	line-height: 28px;
	order: 1;
}

noahtallen · 2019-08-02T16:49:07Z

Wow, nice call! Setting the line height makes the initial jump go away too. One theme fix coming up :)

noahtallen · 2019-08-02T16:53:06Z

Since the issue is in theme styling, I'll close this PR and open a theme PR.

noahtallen · 2019-08-02T17:04:55Z

See Automattic/themes#1181

Utilize RichText for site description block

647363f

- This fixes an issue where the text would jump around when selecting the description textbox in the post editor. - This will also give us more flexibility in the future to expand what users can do with this block.

noahtallen requested review from a team as code owners August 1, 2019 20:25

noahtallen self-assigned this Aug 1, 2019

noahtallen added [Goal] Full Site Editing [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Aug 1, 2019

gwwar reviewed Aug 1, 2019

View reviewed changes

noahtallen closed this Aug 2, 2019

noahtallen deleted the fix/site-description-jump branch August 2, 2019 16:53

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FSE: Fix description block jumping around in the editor #35065

FSE: Fix description block jumping around in the editor #35065

noahtallen commented Aug 1, 2019

matticbot commented Aug 1, 2019

matticbot commented Aug 1, 2019

gwwar Aug 1, 2019

gwwar Aug 1, 2019 •

edited

Loading

noahtallen Aug 1, 2019 •

edited

Loading

noahtallen Aug 1, 2019

gwwar Aug 1, 2019

gwwar commented Aug 1, 2019 •

edited

Loading

noahtallen commented Aug 1, 2019

apeatling commented Aug 1, 2019

gwwar commented Aug 1, 2019 •

edited

Loading

gwwar commented Aug 1, 2019

noahtallen commented Aug 1, 2019

apeatling commented Aug 1, 2019

gwwar commented Aug 1, 2019 •

edited

Loading

noahtallen commented Aug 2, 2019

Copons commented Aug 2, 2019

noahtallen commented Aug 2, 2019

Copons commented Aug 2, 2019

noahtallen commented Aug 2, 2019

noahtallen commented Aug 2, 2019

noahtallen commented Aug 2, 2019

FSE: Fix description block jumping around in the editor #35065

FSE: Fix description block jumping around in the editor #35065

Conversation

noahtallen commented Aug 1, 2019

Changes proposed in this Pull Request

Testing instructions

matticbot commented Aug 1, 2019

matticbot commented Aug 1, 2019

gwwar Aug 1, 2019

Choose a reason for hiding this comment

gwwar Aug 1, 2019 • edited Loading

Choose a reason for hiding this comment

noahtallen Aug 1, 2019 • edited Loading

Choose a reason for hiding this comment

noahtallen Aug 1, 2019

Choose a reason for hiding this comment

gwwar Aug 1, 2019

Choose a reason for hiding this comment

gwwar commented Aug 1, 2019 • edited Loading

noahtallen commented Aug 1, 2019

apeatling commented Aug 1, 2019

gwwar commented Aug 1, 2019 • edited Loading

gwwar commented Aug 1, 2019

noahtallen commented Aug 1, 2019

apeatling commented Aug 1, 2019

gwwar commented Aug 1, 2019 • edited Loading

noahtallen commented Aug 2, 2019

Copons commented Aug 2, 2019

noahtallen commented Aug 2, 2019

Copons commented Aug 2, 2019

noahtallen commented Aug 2, 2019

noahtallen commented Aug 2, 2019

noahtallen commented Aug 2, 2019

gwwar Aug 1, 2019 •

edited

Loading

noahtallen Aug 1, 2019 •

edited

Loading

gwwar commented Aug 1, 2019 •

edited

Loading

gwwar commented Aug 1, 2019 •

edited

Loading

gwwar commented Aug 1, 2019 •

edited

Loading