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

Modifying a block's HTML can cause an unexpected "This block appears to have been modified externally." error #6826

Closed
sandeshjangam opened this issue May 18, 2018 · 11 comments
Assignees
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability Needs Design Feedback Needs general design feedback. [Type] Developer Documentation Documentation for developers

Comments

@sandeshjangam
Copy link
Contributor

sandeshjangam commented May 18, 2018

Describe the bug
If I modify custom block's class name or attribute then "This block appears to have been modified externally." this error is appearing.

To Reproduce

  1. Add block
  2. Save it
  3. Change class or an attribute of a block from code. Just minor change then refresh editor.
  4. Error will appear

Expected behavior
If HTML or attribute changing from code then it should not show this error. Because users value will be same we are just changing the HTML and its class to fix the bugs or change the way of showing.

If this is the natural behavior then it will be very hard to fix block bugs or add a feature in the block because if you touch any part in the code it will throw this error.

Screenshots
https://imgur.com/a/fAPeQ0O

@danielbachhuber danielbachhuber added the Backwards Compatibility Issues or PRs that impact backwards compatability label May 18, 2018
@danielbachhuber
Copy link
Member

Hi @sandeshjangam,

Thanks for the report. This behavior is by design, so I think it's unlikely to change.

@mtias @youknowriad Is there some existing documentation for the rationale behind this design decision?

@danielbachhuber danielbachhuber added this to the Merge Proposal: Back Compat milestone May 18, 2018
@danielbachhuber danielbachhuber changed the title Custom block modify error Modifying a block's HTML can cause an unexpected "This block appears to have been modified externally." error May 18, 2018
@danielbachhuber danielbachhuber added the [Type] Developer Documentation Documentation for developers label May 18, 2018
@sandeshjangam
Copy link
Contributor Author

@danielbachhuber Thanks for considering this.

We need to find something for this as it will create trouble while fixing bugs of the block.

Possible Solution => In editor mode, we will render HTML from block code instead of the saved block HTML.

@mtias
Copy link
Member

mtias commented May 30, 2018

@sandeshjangam @danielbachhuber changes to a block markup (it includes class names and attributes) are meant to be handled through the native deprecated mechanism of blocks. You can check out the core "Quote" block which contains a change in a tag name. This ensures you can smoothly transition a block to a new markup shape without bothering users, but it has to be explicit, otherwise it is recognized as an error — we have no way of knowing if this is an intended change or a relevant bug that would disrupt user's content.

That said, when looking specifically at classes there is room for the thought they should not be considered as relevant changes when comparing source and output. This is debatable, though, and could introduce a level of fragility that is hard to account for. The fact your block is being recognized as having an issue is actually telling you that a portion of your users would be in the previous version of the markup, and that your knew shape doesn't account for them.

Going to close the issue, but feel free to continue the conversation.

@mtias mtias closed this as completed May 30, 2018
@danielbachhuber
Copy link
Member

@mtias Can you explain the rationale behind "Edit as HTML" if you can't edit the HTML?

htmlmode

@danielbachhuber
Copy link
Member

Related #4522 (comment)

@mtias
Copy link
Member

mtias commented May 30, 2018

You can edit in HTML. If you break the validation with your changes you are offered the option of keeping it as an HTML block.

@danielbachhuber
Copy link
Member

If you break the validation with your changes you are offered the option of keeping it as an HTML block.

Oh, I see what you mean:

image

In this case, "Edit as HTML" means "Convert to HTML" block, not "Go Back to Code View". I had assumed the latter, given the label is the same as this:

image

@mtias
Copy link
Member

mtias commented May 30, 2018

Yes, should probably rename to "Keep as HTML" or similar. And the overall writing of the message should be tweaked as well to be less presumptuous in having edited it externally when you haven't :)

@danielbachhuber
Copy link
Member

More conversation in https://wordpress.slack.com/archives/C02QB2JS7/p1527696785000069

Some key bits:

danielbachhuber [9:24 AM] I'm confused in a broader sense: discarding HTML attributes.
Like, in TinyMCE I can switch to HTML mode, write <h2 class="testing">my heading</h2> and the class is persisted when I switch back > to Visual mode. (edited)
However, Gutenberg will either strip class="testing" or bork.
matias [9:27 AM] Blocks have a declared expected shape that’s build from markup source and attributes.
If you are pasting arbitrary HTML and want to preserve it you should paste it in an HTML block
That additional CSS class is being treated as an attribute
We are not inspecting the HTML source to extract it
timothybjacobs [9:30 AM] It is also unclear to the user whether a change they are going to make is going to result in the block being > invalid. For example, why can I add add text to this block here, but I can’t change an attribute?
danielbachhuber [9:30 AM]
Anyways, I'd like to explore further.
matias [9:30 AM]
Well, the current setup is to be as strict as possible with diffing.
There are multiple ways in which the UX of it could be relaxed.
danielbachhuber [9:31 AM]
It seems like the entire flow would benefit from some documentation and consideration.
matias [9:31 AM] From having a threshold — say, above certain amount of char difference — to attempting to identify if something is an > attribute.
It’s super useful for now to be strict.
danielbachhuber [9:32 AM] I imagine. Hopefully there's only a few strategic changes needed to make it more user-friendly (e.g. > automatically recognizing added classes)

@danielbachhuber
Copy link
Member

Flagging as Needs Design because I think this needs a round of UX / design consideration for this use case before it goes to development. #7097 is related.

@mtias
Copy link
Member

mtias commented Jun 28, 2018

Consolidating planned improvements on #7604.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability Needs Design Feedback Needs general design feedback. [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

No branches or pull requests

4 participants