Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buttons: Disable edit as HTML support #49097

Merged
merged 1 commit into from
Mar 20, 2023

Conversation

Mamaduka
Copy link
Member

What?

Resolves #49095.
A similar PRs: #11785, #39318.

PR disabled HTML editing for the Buttons block.

Why?

Editing these wrapper blocks in HTML directly is fragile and prone to breaking the users' content. HTML editing should be left to leaf-blocks.

Testing Instructions

  1. Open a Post or Page.
  2. Insert a Buttons block.
  3. Confirm "Edit as HTML" action isn't available in block Options.

Testing Instructions for Keyboard

Same as general instruction.

Screenshots or screencast

CleanShot 2023-03-15 at 17 33 39

@Mamaduka Mamaduka requested review from talldan and gziolo March 15, 2023 13:39
@Mamaduka Mamaduka self-assigned this Mar 15, 2023
@Mamaduka Mamaduka added [Type] Bug An existing feature does not function as intended [Block] Buttons Affects the Buttons Block labels Mar 15, 2023
@carolinan
Copy link
Contributor

But what is causing this error? Why is the block markup not visible in the input field when the Edit HTML option is selected?

@carolinan
Copy link
Contributor

Removing this option may conflict with #47868

@Mamaduka
Copy link
Member Author

Why is the block markup not visible in the input field when the Edit HTML option is selected?

Which block's markup?

Removing this option may conflict with #47868

Can you give me more details? For example, how disabling can UI action conflict with attribute supports?

The individual Button blocks can still be edited as HTML. This is only disabled for a wrapper.

@carolinan
Copy link
Contributor

carolinan commented Mar 16, 2023

Why is the block markup not visible in the input field when the Edit HTML option is selected?

Which block's markup?

I did not understand why editing this block would be more prone to problems compared with for example the image block which has many more lines of code. I also do not know what the term "leaf blocks", that is quoted above, refers to.

Without the PR applied, (6.2 current nightly, no Gutenberg) I added the block in the editor and selected the Edit as HTML option.
But the textarea that is supposed to contain the code was empty, so it is not surprising that the HTML edit breaks.
I re-tested it again now and I realized I had selected the button block, not the buttons.
Button block HTML text area is empty

With the PR applied:
With PR: Button block HTML text area is empty

Adding anything inside the textarea causes a block validation error.

So the questions are, why is it empty and breaking?, are there any benefits to fixing the underlying issue instead of removing it?
If you think removing it is the best then I will not block it, but I think it needs to be removed from both blocks.

Removing this option may conflict with #47868
Can you give me more details? For example, how disabling can UI action conflict with attribute supports?
The individual Button blocks can still be edited as HTML. This is only disabled for a wrapper.

You are right, I mixed up the block names and got confused about the markup being different if there is no text content in the button.

@Mamaduka
Copy link
Member Author

I think you're editing a Button block without content; that's when you get empty input. I believe that's by design.

The "leaf block" refers to the inner block, in this case.

So the questions are, why is it empty and breaking?, are there any benefits to fixing the underlying issue instead of removing it?

The bug has been around for a while now, as you can see by the linked similar issues. I agree this is a temporary solution, but we already use it in practice.

Parsing markup again on the block mode switch has its downside. See #43219.

Screenshot

CleanShot 2023-03-16 at 10 56 38

@carolinan
Copy link
Contributor

But back to the PR; I still think that it will not be enough of an improvement if only the parent buttons block is fixed, because it is too easy to cause a block validation error also with the button block.
All it takes is selecting HTML edit before you add a text and then entering anything inside the text area.

@Mamaduka
Copy link
Member Author

I think that's the general problem with the "Edit as HTML" or "Code Editor" modes. It's easy to mess up the blocks.

Probably not something I can resolve here and now 😢

@carolinan
Copy link
Contributor

All I am suggesting is adding

"supports": {
		"html": false,
		

to block.json for the button block as well as the change you already made to buttons
(And regenerating the docs)

@Mamaduka
Copy link
Member Author

That was initially proposed in this PR #20948 (comment) and then reverted.

The current policy seems to disable it for container blocks (mostly).

Maybe @talldan or @mtias have more context.

@talldan
Copy link
Contributor

talldan commented Mar 20, 2023

I think the decision was to keep it in place on the button block as there should be no difference to other static 'leaf' blocks when using 'Edit as HTML'.

Why is the block markup not visible in the input field when the Edit HTML option is selected?

This seems like a separate bug. If you compare it to a header block, you at least get the block markup when there's no text:
Screen Shot 2023-03-20 at 2 32 58 pm

It shouldn't be completely empty for the button block.

Also, when you click 'Edit visually' on the toolbar of a button block, the text '[Object object]' briefly flickers into view in place of the block content, so I think there's a problem somewhere.


I think that's the general problem with the "Edit as HTML" or "Code Editor" modes. It's easy to mess up the blocks.

Probably not something I can resolve here and now 😢

I never fully understood the feature, considering it always caused validation errors. It's possibly something that was more relevant in the early days when users were switching from the classic editor. Now you can do so much with just the block editor interface, is it worth re-evaluating it?

Perhaps making it available via a preference/setting that defaults to off?

@Mamaduka
Copy link
Member Author

Thank you, @talldan!

I agree. The Button block has a separate bug, and I'll try to look into that shortly.

I never fully understood the feature, considering it always caused validation errors. It's possibly something that was more relevant in the early days when users were switching from the classic editor. Now you can do so much with just the block editor interface, is it worth re-evaluating it?

Removing the feature that's been around this long would be hard. Maybe we should only allow it for Admin users.

P.S. What do you think about this solution? Should we go with it?

Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P.S. What do you think about this solution? Should we go with it?

Sounds good. I wonder about List and Quote, which also now have this issue 😄

@Mamaduka
Copy link
Member Author

Mamaduka commented Mar 20, 2023

We can apply the same "solution" for other parent blocks 😅

P.S. The Button block displays an empty input because it renders nothing when there's no text specified. See #29781.

@talldan
Copy link
Contributor

talldan commented Mar 20, 2023

P.S. The Button block displays an empty input because it renders nothing when there's no text specified. See #29781.

Well spotted. I guess it's fine to keep it as it is, it just means anyone editing as HTML has to type the whole block html in when there's no text.

@Mamaduka Mamaduka merged commit 196dc1f into trunk Mar 20, 2023
@Mamaduka Mamaduka deleted the update/buttons-disable-html-editing branch March 20, 2023 08:07
@github-actions github-actions bot added this to the Gutenberg 15.5 milestone Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Buttons Affects the Buttons Block Needs User Documentation Needs new user documentation [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Opening Edit HTML on container blocks shows error "This block contains unexpected or invalid content"
4 participants