Add a Attempt Fix button for paragraphs blocks that are invalid #4813
Conversation
Polish the font, the phrasing, margins.
|
Pushed some visual and verbiage polish:
Otherwise this seems good to me. I wish we could do with less text that was more easily readable, but I can't quite think of anything. |
Remove the overwrite button in favor of "Attempt Fix" which should resolve any issues with paragraphs and seems like a better solution to restore the content verse delete. Having four buttons to choose from for an invalid block seems like too much to understand the different options.
| 'your changes.' | ||
| ), defaultBlockType.title, htmlBlockType.title ) }</p> | ||
| <p> | ||
| <Button | ||
| { block.name === 'core/paragraph' && ( |
There was a problem hiding this comment.
I wonder if we want to target the block explicitely here or use getDefaultBlock.
Maybe a nicer way to handle this is to avoid the message entirely cc #4874
There was a problem hiding this comment.
Can we do both? I didn't see #4874 that looks good, this code would only be a last resort if it can correct a P block ahead.
There was a problem hiding this comment.
I think we do get #4874, there's no need to special-case the paragraph block anymore because the most common way to make it invalid will be gone. So this button would become useless.
There was a problem hiding this comment.
yeh, makes sense. I already merged this one. So should I simply create a new PR to back out the change?
There was a problem hiding this comment.
Sounds good to back out the change since handling p-less text as deprecated should fix the fundamental issue.
Separately, we should do an iteration on the invalidation screen, as "Overwrite" is indeed meaningless. We also have the ability to interpret as HTML (as if you had copied and pasted raw HTML which we then convert to blocks) which might be a much better default.
There was a problem hiding this comment.
I like converting to HTML as a primary (recommended) action because it preserves the content (content loss impossible), immediately shows what may be wrong by showing the previously invisible HTML, and the user could continue from there to convert to blocks (we should add a convert to blocks action to the HTML block).
There was a problem hiding this comment.
We could also give a "convert to blocks" option in here, but that's again another button. I'm unsure how "attempt to fix" would be any different from "convert to blocks". I would also remove "convert to classic" in favour of "edit as HTML block"
There was a problem hiding this comment.
I'm going to make a PR for this.
|
@jasmussen @mtias Do you think we could make one of these buttons primary? As a user, I'm overwhelmed with the options here, and I don't feel like I would understand most of them without being involved with development. |
|
Perhaps we should even go one step further — only have a single button action, and have the rest of the actions be in a menu you have to open. |
|
@jasmussen @iseulde my merged commit I did reduce down to 3 buttons, removing Overwrite. I'm all for less decisions for the user. I'm not a fan of Overwrite button at all because I don't know what it means and it really just deletes whatever content is there and gives an empty block. I'm not sure that behavior is ever what a user wants to do. |
|
The "Assign Fix" button was backed out in #4874 which solves the problem so a paragraph should not create an invalid block if missing P tags, it auto corrects it. Design changes can be addressed in a new ticket. |
| @@ -1,4 +1,4 @@ | |||
| .editor-warning { | |||
| .edit-post-visual-editor .editor-warning { | |||
There was a problem hiding this comment.
Why was this change made? It regressed styling on the application-wide error.
There was a problem hiding this comment.
Also, should avoid targets that are outside the scope of the component.
There was a problem hiding this comment.
Yeah, probably worth moving the post-editor-specific styles to a component in that directory.
| background-color: $white; | ||
| border: 1px solid $light-gray-500; | ||
| text-align: center; | ||
| line-height: $default-line-height; | ||
| box-shadow: $shadow-popover; | ||
|
|
||
| p { | ||
| font-family: $default-font; |
There was a problem hiding this comment.
The addition of these styles makes the identical styles below in .editor-visual-editor & p { redundant.
Description
The paragraph tags are notorious for getting lost, especially with various editors and markdown formatting that does strange things like autop and removep. This change attempts to recover those lost paragraph tags and return them to their rightful place.
The current option of "Overwrite" seems rather rude to just delete the content.
How Has This Been Tested?
Create a paragraph block in Gutenberg and then in Code Editor mode, or if you prefer in the Classic Edtior, remove the
<p>tags but keep the Gutenberg block comments. When you toggle back to Visual mode, you will get the invalid block. Clicking Overwrite will delete the blocks.With this change, an Attempt Fix button would be added when click will maintain the content, making the user much happier than having the content deleted.
I hedged my bets by calling the button "Attempt Fix" - if this seems valid, I would probably recommend just naming it "Fix" and remove the Overwrite button. The way the content gets set, I can't really think of a scenario that this fix wouldn't work, open to suggestions and thoughts.