Blocks: update warning style

[johngodley]

Description

Update the block warning style, moving the action buttons inline with the text, and adding a grey scrim. This prepares the warning component for the addition of a dropdown options menu. Discussion and reasons behind these changes can be found in #7604

The block warning is shown in two situations:

• A block crashes and a <BlockCrashWarning /> component is shown
• A block contains unexpected content and a <BlockInvalidWarning /> is shown

In addition to style changes some text has been changed:

• The Edit as HTML button is now Keep as HTML to better reflect the action
• The message This block appears to have been modified externally. is now This block has been modified externally. to remove general vagueness

These items of text are likely to change in the future as the validation experience is improved, but for now should help with confusion.

Also to note that InvalidBlockWarning has been renamed to BlockInvalidWarning so it matches the other block components.

How has this been tested?

Existing unit tests cover the changes to the HTML.

To manually test the block validation message:

• Create a paragraph
• Edit as HTML and remove the trailing </p> to trigger the block validation message
• Verify that the message changes from:
  
  to:
  
• The buttons are right aligned and a grey scrim overlays the block content
• Verify that as the page width decreases the warning adjusts the buttons beneath the text (still left aligned):
  
• Verify that the buttons continue to work as before

To test the block crash warning message:

• From a developer console type wp.blocks.registerBlockType( 'myplugin/mr-crashy', { title: 'Mr. Crashy', category: 'common', icon: 'warning', edit() { throw new Error(); }, save() { return ''; } } );
• Add Mr Crashy block to a post
• Confirm that the crash warning has changed from:
  

To:

• The warning now matches the width of a block

Types of changes

This is a non-breaking change. The block warning functionality doesn't change.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.

[karmatosed]

Can I ask from a design perspective what happens with translations causing this to be on 2 lines? It seems it would throw everything off quite a lot. Also what happens on smaller screens?

[johngodley]

Can I ask from a design perspective what happens with translations causing this to be on 2 lines? It seems it would throw everything off quite a lot. Also what happens on smaller screens?

It's a bit hidden in the text above, but this screenshot shows what happens when the width decreases:

The same will happen if the text or buttons are bigger:

Once it wraps it behaves the same as the existing style, but left aligned.

[karmatosed]

Approving based on design. I do wonder if the fact the buttons move is great for consistency but we can explore other solutions once in. Having a giant modal isn't great either so this at least solves that one.

[johngodley]

@aduth you asked some questions in #7886 regarding the changes here. Does this PR answer them?

Specifically:

increasing the z-index for the block warning

Joen adjusted the z-index for all block warnings. As far as I'm aware (and admittedly I know very little of Gutenberg at this stage), there's only an invalid block and crashed block warning. If that's the case then it shouldn't have any other effect, although I'm basing that on Joen's knowledge.

Why was this moved?

As mentioned in f04f95a it was moved to bring the crash warning to the same level in the DOM as the invalid block warning, allowing the style changes to apply to them both. Previously it was a level higher.

[aduth]

Joen adjusted the z-index for all block warnings. As far as I'm aware (and admittedly I know very little of Gutenberg at this stage), there's only an invalid block and crashed block warning. If that's the case then it shouldn't have any other effect, although I'm basing that on Joen's knowledge.

The original worry was that we tend to think of only these ones as the only instances, but it was rewritten at one point to intentionally be generic, so we should be conscious of unintended impacts (including for future usage). Since it's increasing a z-index (and only slightly so), I doubt it's going to break much, but I'm still curious why it's necessary. And if it's necessary for only the block usage, whether it should only be applied to the block-specific variant (even just for clarity for the next maintainer)

As mentioned in f04f95a it was moved to bring the crash warning to the same level in the DOM as the invalid block warning, allowing the style changes to apply to them both. Previously it was a level higher.

This seems okay. Which styles specifically, though?

[johngodley]

I'm still curious why it's necessary

I think Joen will need to answer that, although he is away so it will be a while. If this is something that you'd rather wait for then that's fine. If it makes sense to merge now and adjust later with Joen's info then 👍

Which styles specifically, though?

.editor-block-list__block .editor-warning. I'm sure it would be possible to get the style looking the same without this DOM change, but this has simpler CSS and puts the warnings at the same place and using the same style so seems like a sensible thing, and should help for any future warnings.

[karmatosed]

I would +1 to merging it now and iterating.

[aduth]

If there are follow-up items, we should create an (ideally self-assigned) issue.

Otherwise, not a blocker for me.

[aduth]

I mentioned at #7886 (comment) that this component is used for more than just the warning for the single block in a list. It's also used for the top-level error handler, which this change broke. See #8677.