Block warning: add ellipsis menu and convert to classic block

[johngodley]

Description

Add the ability for a block warning message to show a list of additional actions that are hidden behind a dropdown ellipsis menu. This will be used to expand the options available to a user when an invalid block is detected.

The 'convert to blocks' option is copied into this menu, along with a new 'convert to classic block' option. This matches the mockup in #7604. More options will be added in future PRs

How has this been tested?

Extra unit tests have been added to verify when the ellipsis menu appears.

npm run test warning

Types of changes

A non-breaking new feature is added to an existing component. The Warning component is used by:

• BlockInvalidWarning
• BlockCrashWarning

To test BlockInvalidWarning:

• Create a paragraph
• Edit as HTML and remove the trailing </p> to trigger the block validation message
• Verify that the invalid block message is displayed with a new ellipsis menu:
  
• Verify that reducing the page width causes the primary buttons to move onto the next line, but the ellipsis menu remains on the right:
  
• Verify that clicking 'convert to block' from the ellipsis menu performs the same action as the 'convert to block' button
• Verify that clicking 'convert to classic block' converts it to a classic block

To test BlockCrashWarning:

• 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
• Verify that the crash message is displayed without changes:
  

Checklist:

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

[jasmussen]

Very nice work, I pushed some polish:

I also removed the styles for the contents of the navibable menu. Can I kindly ask you to take a look at the more menu and perhaps steal a bit from there? We have some components for menuy groups, menu labels and even menu items. If we reuse those, we get the visual styles, as well as their accessibility for free.

[johngodley]

The overall style was moved into another PR, and the code here updated to use the 'more menu' as mentioned by Joen. I've added a slight CSS tweak to better vertically align the ellipsis button

[jasmussen]

Very cool, what's the status of this? Does it need a wider Gutenberg review? Do you need anything from me to ship this?

[youknowriad]

Looking good, just left some small notes.

[youknowriad]

Could this be secondaryActions (to match the "primaryActions" prop)?

[youknowriad]

The edit-post-* className seems useless right? (they don't follow our guidelines neither)

[johngodley]

It's the same class used in https://github.com/WordPress/gutenberg/blob/master/edit-post/components/header/more-menu/index.js, making the ellipsis vertical rather than horizontal. I can file a separate issue if that also needs changing to match the guidelines?

[youknowriad]

I don't think we should rely on classnames styled in other components. We should aim to reuse components instead of classnames instead.

So two options:

• I'm fine duplicating the styles (if not too much) and using the right classnames here editor-warning__*
• Creating a generic component with these styles but in that case, this component should be meaningful. Maybe it could be a prop of Dropdown instead?

[johngodley]

@youknowriad thanks for looking. All good points.

For the purpose of this PR I've copied the appropriate styles to this component - it's pretty minimal.

While looking at the CSS I noticed the original component has what appears to be dead CSS as well as problems on mobile (that only affect the other component).

I'll investigate that separately and if it makes sense, refactor with this component while fixing any problems.

[youknowriad]

LGTM 👍 Nice work

[aduth]

This shouldn't have been merged.

• It broke backwards-compatibility with the Warning actions prop by renaming it to primaryActions. This should have been deprecated, with support for actions kept for two versions, documented in docs/deprecated.md and noted in packages/editor/CHANGELOG.md
• Related to the previous point, I assume it broke both ErrorBoundary and the "validate multiple use" hook, both of which render Warning with the now-non-existant actions prop

  • gutenberg/packages/editor/src/components/error-boundary/index.js

    Lines 52 to 54 in 6259b23

    	<Warning
    	className="editor-error-boundary"
    	actions={ [

  • gutenberg/edit-post/hooks/validate-multiple-use/index.js

    Lines 75 to 77 in 6259b23

    	<Warning
    	key="multiple-use-warning"
    	actions={ [

[youknowriad]

Good catch @aduth I missed that :(