Improve Custom HTML Block so preview mode persists on save.

[mauteri]

What?

This PR is to maintain preview status on HTML Block when saved, so on refresh if the HTML block was saved in preview mode, it will load in preview mode. If it was saved in raw HTML mode, it will load in raw HTML mode.

Addresses #40913

Why?

Raw HTML is sometimes unavoidable and can be scary or jarring to the enduser. In order to maintain a what-you-see-is-what-you-get feel in the block editor, it is important to have the option to see Raw HTML in preview mode where it can look more at home with the rest of the blocks/content in the editor.

How?

This is just a small change to the block making preview an attribute of the Custom HTML block. Now its state is saved with the block and can easily be loaded in preview mode or raw HTML mode.

Testing Instructions

1. Create a new post or open an existing one.
2. Add a new or edit an existing Custom HTML Block.
3. On load it should be RAW HTML, change it to preview mode, save, and refresh.
4. It should load in preview mode. Change it to raw HTML mode and save.
5. It should load in raw HTML mode.

Testing Instructions for Keyboard

N/A. No changes were made to the general UI of the block.

[mauteri]

Just realized there was an older/abandoned PR for this here: #40913

Reading through some comments I saw there was another thought of making the Raw HTML visible on select and preview visible when not selected. I really like that idea and can adjust my PR to handle it that way if more folks think that is the better approach. Thx!

Update: Added a new PR for using on select rather than buttons in case this is the preferred approach: #46836

[alexstine]

@mauteri This works as described. I really like this solution over the linked PR.

[mauteri]

@alexstine anything else needed before this can be merged? The failing pipeline for React Native E2E Tests (Android) isn't required and doesn't appear to be related to my code, but not sure if this is why PR hasn't been merged. Let me know. Thx!

[kohheepeace]

+1 for this feature. I also prefer isPreview instead of preview for naming convention, but this is just a my opinion.

[mauteri]

+1 for this feature. I also prefer isPreview instead of preview for naming convention, but this is just a my opinion.

I can make this change. is* methods make it clear that it's a boolean. I'll get that updated later today. Thx!

[mauteri]

Updated for @kohheepeace suggestion and did small cleanup. Let me know if anyone else has anymore feedback. Thx!

[alexstine]

@carolinan Do you see anything else that should be addressed for this change? I tested the functionality and it works well. I know you have done some work with blocks and this is a newer area of the editor dev for me.

Thanks.

[kohheepeace]

Since there is a change in the attributes, a deprecation of the block necessary in this case isn't it?

Just modify test/integration/fixtures/blocks/core__html.json loses old core/html block tracking right ?

I am not completely understand the deprecation of the block, but I am curious about this.

[carolinan]

The PR works well in my test.
I don't think this needs a deprecation because the old markup, where the attribute is not used, is still valid.
Deprecations are difficult so it would be good to get another second opinion...

[mauteri]

Awesome, thanks @carolinan!

[alexstine]

Maybe one more from @talldan or @tellthemachines ? Not sure how depreciation works in blocks either.

[mauteri]

Maybe one more from @talldan or @tellthemachines ? Not sure how depreciation works in blocks either.

Hi @alexstine not sure why deprecation would be needed or considered here? isPreview is false by default so there is no negative effect to any existing Custom HTML block in the wild, ie, when isPreview is false the block looks exactly as it does currently so no risk of breakage.

Maybe I'm not fully understanding, so if I am please let me know what I'm missing or not understanding. Thx!

[kohheepeace]

Sorry. I was mistaken. 😿

When I run the following command

npm run test:unit test/integration/full-content/

I got a validation error, so I thought I needed a new deprecated-* file.

However, this Validation error was corrected by the following command

npm run fixtures:regenerate

which updates test/integration/fixtures/blocks/core__html.json.

So, I think this PR is to be merged 🚀🚀🚀🚀

cc: mention to @talldan @glendaviesnz who seems to be familiar with block deprecation just in case.

[talldan]

Thanks for pinging me. I don't think it requires a deprecation–only changes to block save output requires a deprecation. This is purely an editor change. The easiest way to check is to add an HTML block to a post using trunk, then checkout and build this branch and reload that post. If the block has a validation error, you'll know that it needs a deprecation.

On the feature itself, I'm personally unsure if this should be saved as an attribute. One consideration is that when the Site Editor is in browse mode, the HTML block should now probably always show its preview instead of its code. It might be best to start with that feature first. I'm not sure if there's scope to introduce a browse mode to the post editor too, that's an interesting question.

The issue hasn't had much feedback from design contributors, so I'd suggest seeking that first before shipping an implementation.

[carolinan]

Maybe it is as simple as while in preview mode, the text of the other button could be "Edit HTML" instead of "HTML"

[mauteri]

Maybe it is as simple as while in preview mode, the text of the other button could be "Edit HTML" instead of "HTML"

I think that makes is very clear.

[mauteri]

Could be just Edit and Preview to keep things short and sweet since we know this is the HTML Block.

[wilnau-design]

+1 Need this so badly!

[mauteri]

+1 Need this so badly!

I've been meaning to get back to this! I'll look to continue the effort if someone else doesn't pick it up.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @kohheepeace, @wilnau-design, @zachalig.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: kohheepeace, wilnau-design, zachalig.

Co-authored-by: mauteri <[email protected]>
Co-authored-by: alexstine <[email protected]>
Co-authored-by: carolinan <[email protected]>
Co-authored-by: talldan <[email protected]>
Co-authored-by: jasmussen <[email protected]>
Co-authored-by: colorful-tones <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[mauteri]

Hi, I finally came back to this pull request to hopefully finish things up with the feedback received. In the latest updates I did the following.

• Resolved conflicts
• Changed "HTML" to "Edit" (see comment above)
• Added a hover effect to block when in preview mode per @jasmussen's suggestion.
  Let me know if this is good to go or if there are additional changes you'd like to see before merging. Thanks!

[mauteri]

Hi, I just wanted to follow up on this PR as I made updates some time ago. Is there any additional feedback or could this code be merged? Thanks!

[zachalig]

+1 Would love to see this merged. We have some official HTML footers I have to use on all sites and it would be nice to be able to see them represented properly in the editor.

[mauteri]

Hi @jasmussen I just wanted to bring this PR to your attention again. I added the hover feature like you illustrated above and curious if this is enough approve and merge. Thanks!

[jasmussen]

Thanks for the ping again, thanks for the patience. Thanks also for trying the purple hover style. There's still something to that idea, around making editability clearer. However in trying the branch just now, it doesn't feel as consistent or similar with synced patterns and template parts, as I had hoped.

So I think we should remove those hover styles again.

I don't want to be the blocker for this PR, so I'll defer to others, including @carolinan, on whether this PR can land as-is without that hover style. But the remaining issue I found was with interactivity and links. Quick GIF:

Shown here, simple HTML markup in the preview state to show "Hello world". When you click to preview it, and then click the link, the website linked loads inside a tiny sliver of an iframe for rendering the code.

Visiting the link both feels a bit unexpected since you're in an editor where that's not what happens with other links, it's especially unexpected when the result loads in a tiny iframe. Finally you could write markup that's just one big link for the whole visible area, and then you wouldn't be able to click the block to select it, you'd instead click the link to go there.

The solution I could see here is to wrap the entire preview inside a Disabled component (docs). That would not only fix the issue with following links, it would let the block itself be selectable even if it was in a preview state.

The tradeoff would be that any interactivity you're building in the custom HTML would only be testable on the frontend. But maybe that would be fine? After all, if you want this feature so you can add a linked SVG logo, I suspect you'd want to be able to click it to select it.

Let me know what you think! And thanks for contributing.

[mauteri]

Thanks @jasmussen! I'll review your notes above and make some adjustments. I'll also remove the hover state. Thanks!

[alexstine]

@mauteri @jasmussen Sorry, but please do not use the Disabled component. I'm not going to waste the time again to go through why it is not accessible but it should have never been added in the first place. TLDR: It uses the inert attribute which makes it inaccessible to keyboard/screen reader users. More here.

#54369

Thanks.

[jasmussen]

Okay, understood.

I'm unsure how to move this PR forward, and I'll defer to the rest of you.

[mauteri]

@mauteri @jasmussen Sorry, but please do not use the Disabled component. I'm not going to waste the time again to go through why it is not accessible but it should have never been added in the first place. TLDR: It uses the inert attribute which makes it inaccessible to keyboard/screen reader users. More here.

#54369

Thanks.

Cool thx @alexstine i'll remove that as well.

[mauteri]

@alexstine @carolinan either of you having issues with lint-staged-typecheck? I tried everything I can think of and it's failing when I try to commit (in places outside of my code). Any thoughts? Thanks!

[alexstine]

Maybe merge upstream trunk into your branch and npm i && npm ci.

[mauteri]

Thanks @alexstine, it was up to date with trunk. I also tried blowing away node_modules and re-installing. I didn't run npm i && npm ci specifically, but that appeared to work today. 🤷🏻‍♂️ Thx again.

[mauteri]

@alexstine @jasmussen I think this is ready again. Disabled was removed per Alex, and the CSS hover was removed as well. The link issue that was pointed out is an issue with the current block, so not a new issue. I think how that is solved is open for discussion, but maybe not in the scope of this ticket. Basically, this ticket is just to keep the preview view persistent on save, which I think many people want since it looks more natural in the block editor than a a bunch of HTML.

Is there anything else preventing this from being merged? Thanks!

[colorful-tones]

I just tested this and it is working as explained in the testing instructions.

However, I am a bit concerned with the overall user experience challenges.

The primary goal is to persist the last state for the HTML block that the user saved, either: Preview or HTML. During my testing I utilized the User Switching plugin to test how the state persisted between different users.

I edited a post as an Admin and saved the state as 'Preview'. I then switched to an Editor role user, edited the same post, and the 'Preview' state was offered. I was mostly curious how this state might impact different users (and their experiences). It is difficult to assert whether this PR and its primary goal are impacting (positively or negatively) an overall experience in the editorial flow.

Ultimately, it does solve the persistence of state for the HTML block, but it could be misleading for users to be unaware that the HTML block is actually editable if always offered the Preview state.

I would encourage further insight and user experience feedback from other folks.

[mauteri]

Thanks @colorful-tones for your feedback. It was great chatting with you at WordCamp Montclair yesterday, hope you had a good time.

Ultimately, it does solve the persistence of state for the HTML block, but it could be misleading for users to be unaware that the HTML block is actually editable if always offered the Preview state.

I could see some initial confusion when content looks like it should be able to be editable while in preview mode. I'm thinking text content in particular. For a person that uses and understands block editor though, selecting the HTML block does clearly mark it as Preview in the UI and shows the option to Edit as well. I would think this is enough of an indicator on selection how to make changes to the block to the user. I still strongly think the clean look of the WYSIWYG keeping HTML Block in preview mode improves the experience overall. Above we were experimenting with the preview to indicate that the block is in a preview mode. Maybe we need to revisit that with some UX direction?