WordPress
Improve Custom HTML Block so preview mode persists on save.
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.
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
- Create a new post or open an existing one.
- Add a new or edit an existing Custom HTML Block.
- On load it should be RAW HTML, change it to preview mode, save, and refresh.
- It should load in preview mode. Change it to raw HTML mode and save.
- It should load in raw HTML mode.
Testing Instructions for Keyboard
N/A. No changes were made to the general UI of the block.
Just realized there was an older/abandoned PR for this here: https://github.com/WordPress/gutenberg/issues/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: https://github.com/WordPress/gutenberg/pull/46836
@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!
+1 for this feature. I also prefer isPreview instead of preview for naming convention, but this is just a my opinion.
+1 for this feature. I also prefer
isPreviewinstead ofpreviewfor 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!
Updated for @kohheepeace suggestion and did small cleanup. Let me know if anyone else has anymore feedback. Thx!
@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.
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.
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...
Maybe one more from @talldan or @tellthemachines ? Not sure how depreciation works in blocks either.
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!
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.
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.
Hey, thanks for the PR, this is exciting. I wanted to share some historical feedback that the reason we haven't done this in the past, is that it might not be clear to someone editing why an HTML block may not be directly editable — i.e. it might just appear static, when it just requires you to go back to editing mode.
So if we are to land this we need to pair it with some some other indication that it's not editable unless you go into code.
Here's a GIF of the state of this branch, showing the insertion of a custom HTML block, then pasting in an SVG of the WordPress logo, then previewing, saving, and reloading:
Doesn't have to be this PR since it's an existing issue, but it would be good to add code to the custom HTML to hide the border on the preview iframe that's currently visible in an inset style:
As far as additional indication that the block means to be editable, I'd love input from @WordPress/gutenberg-design. But one idea is to leverage the same principle used for template parts and reusable blocks, which is to show an overlay on hover:
I think various mockups have included an explicit "Edit" button as an overlay when the block is selected. Probably not needed immediately, but something to consider.
Maybe it is as simple as while in preview mode, the text of the other button could be "Edit HTML" instead of "HTML"
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.
Could be just Edit and Preview to keep things short and sweet since we know this is the HTML Block.
+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.
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.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
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!
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!
+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.
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!
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.
Thanks @jasmussen! I'll review your notes above and make some adjustments. I'll also remove the hover state. Thanks!
@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.
https://github.com/WordPress/gutenberg/issues/54369
Thanks.
Okay, understood.
I'm unsure how to move this PR forward, and I'll defer to the rest of you.
@mauteri @jasmussen Sorry, but please do not use the
Disabledcomponent. 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.https://github.com/WordPress/gutenberg/issues/54369
Thanks.
Cool thx @alexstine i'll remove that as well.
@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!