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

Block options: disable mode toggle when block is in warning state #7886

Closed
wants to merge 1 commit into from

Conversation

johngodley
Copy link
Contributor

@johngodley johngodley commented Jul 11, 2018

Description

If you put a block into a warning state the block option 'Edit visually' and 'Edit as HTML' are still available and toggle state, but have no outward effect on the block.

To avoid user confusion this PR disables the mode toggle option when a block is showing a warning. As soon as the warning is cleared the option is restored.

another_post___ wordpress_latest _wordpress

Fixes #7743

How has this been tested?

Additional unit tests have been added to test the mode toggle enable/disable.

Manually test that the 'edit visually' and 'edit as HTML' options are removed from the block 'more options' menu:

  • Add a paragraph block, edit as HTML, and remove the trailing </p> to trigger an invalid block warning
  • Access the block 'more options' menu and verify that the 'edit visually' and 'edit as HTML' options are removed (see above screenshot)
  • 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 crashed block 'more options' also does not show 'edit visually' and 'edit as HTML'

Types of changes

This PR changes the behaviour of an existing feature.

Checklist:

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

@johngodley johngodley added [Feature] Blocks Overall functionality of blocks [Status] In Progress Tracking issues with work in progress labels Jul 11, 2018
@ZebulanStanphill
Copy link
Member

Would it make more sense to still show the options, but have them be disabled?

@aduth
Copy link
Member

aduth commented Jul 16, 2018

Note this just adds the capability for additional actions, but doesn’t actually include any

YAGNI.

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Aside: This pull request includes a few too many unrelated changes that's made it more difficult than it should be to review in tandem.

@@ -21,6 +20,7 @@ $z-layers: (
'.components-popover__close': 5,
'.editor-block-list__insertion-point': 5,
'.editor-inserter-with-shortcuts': 5,
'.editor-warning': 6,
Copy link
Member

Choose a reason for hiding this comment

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

Are we increasing the z-index for the block warning specifically, and if so, will it have negative consequences on any other "EditorWarnings" present in the application? It was meant to be made separate for general reusability, and thus we should be aware of cascading effects on other usage. Alternatively, we could create a separate z-index for the specific block warning.

@@ -558,8 +559,8 @@ export class BlockListBlock extends Component {
uid={ uid }
/>
) }
{ !! error && <BlockCrashWarning /> }
Copy link
Member

Choose a reason for hiding this comment

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

Why was this moved?

@johngodley
Copy link
Contributor Author

@aduth this PR is a work in progress and contains the contents of other PRs, specifically #7866 which is handling the changes you commented on. Once the other work is finished I'll remove it from this one so it just contains the relevant code.

@SuperGeniusZeb possibly, I'll need to consult with a designer

@johngodley johngodley mentioned this pull request Jul 17, 2018
4 tasks
@johngodley johngodley changed the title Block options: disable mode toggle when block is in warning state WIP Block options: disable mode toggle when block is in warning state Jul 17, 2018
@johngodley johngodley force-pushed the fix/block-warning-edit-html branch 3 times, most recently from 86c4de7 to c0b9f6c Compare July 29, 2018 12:54
@johngodley
Copy link
Contributor Author

Rebased all the cruft out and went with the suggestion from @SuperGeniusZeb to disable the item, as shown in the updated screenshot. It should be good to go now.

There's a Travis CI e2e failure which seems unrelated to this PR

@johngodley johngodley changed the title WIP Block options: disable mode toggle when block is in warning state Block options: disable mode toggle when block is in warning state Jul 29, 2018
@johngodley johngodley removed the [Status] In Progress Tracking issues with work in progress label Jul 29, 2018
@johngodley johngodley force-pushed the fix/block-warning-edit-html branch from c0b9f6c to 86dc382 Compare July 31, 2018 07:35
@johngodley johngodley force-pushed the fix/block-warning-edit-html branch from 86dc382 to afb6d22 Compare August 11, 2018 09:57
Adds a `canEdit` prop to the block settings menu that determines whether the mode toggle menu option is enabled.

When a block is in an error state (crashed or invalid content) then the mode toggle is disabled as it has no effect.
@johngodley
Copy link
Contributor Author

Closing in favour of #9088

@johngodley johngodley closed this Aug 17, 2018
@johngodley johngodley deleted the fix/block-warning-edit-html branch August 17, 2018 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants