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

Add new Insert Before/After menu items and shortcuts #8909

Merged
merged 8 commits into from Aug 15, 2018

Conversation

Projects
None yet
4 participants
@talldan
Copy link
Contributor

commented Aug 13, 2018

Description

Closes #7297

Adds the following keyboard shortcuts for those actions:

  • Option+Cmd+T (Mac) / Ctrl+Alt+T - Insert Before
  • Option+Cmd+Y (Mac) / Ctrl+Alt+Y - Insert After

How has this been tested?

  • Tested use of MenuItem and shortcut across browsers on Mac (Chrome, Safari, Firefox, Opera)
  • Tested use of MenuItem and shortcut across browsers on Windows (Chrome, IE11, Edge, Firefox, Opera)
  • Tested with a block that uses InnerBlocks with a template lock

Screenshots

MenuItems
screen shot 2018-08-13 at 4 33 36 pm

Shortcut Help
screen shot 2018-08-13 at 4 33 01 pm

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

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

@talldan talldan added this to the 3.6 milestone Aug 13, 2018

@talldan talldan self-assigned this Aug 13, 2018

@talldan talldan requested a review from jasmussen Aug 13, 2018

@jasmussen
Copy link
Contributor

left a comment

Screenshot:

screen shot 2018-08-13 at 11 24 33

Love it. Works as intended.

Might want a code sanity check by @noisysocks, but otherwise, ship it!

@talldan

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2018

Great - I still need to do a bit of work on the template locking side of things. I'll do that first and then get a code review

@talldan talldan force-pushed the add/insert-before-after-menu-items branch from afd1b99 to 88d49f8 Aug 14, 2018

@talldan talldan requested a review from noisysocks Aug 14, 2018

@talldan

This comment has been minimized.

Copy link
Contributor Author

commented Aug 14, 2018

Ready for review now :)

talldan added some commits Aug 13, 2018

Disallow block settings actions that can be triggered through shortcu…
…ts and should not be possible when template locking is active
Tidy up comments
- Add full-stops
- Remove comment that no longer seems relevant (component is not a DropDown)

@talldan talldan force-pushed the add/insert-before-after-menu-items branch from 699775b to e57f842 Aug 14, 2018

@noisysocks
Copy link
Member

left a comment

Code looks fab, and everything works great in my testing!

I tested template locking using this custom post type:

register_post_type( 'test_type', array(
	'label' => 'Test',
	'public' => true,
	'show_in_rest' => true,
	'template_lock' => 'insert',
	'template' => array(
		array( 'core/image' ),
		array( 'core/quote' ),
		array( 'core/image' ),
	),
) );

And this custom block:

registerBlockType( 'test/block', {
	title: 'Test block',
	category: 'common',
	save: () => null,
	edit() {
		return (
			<wp.editor.InnerBlocks
				templateLock="insert"
				template={ [ [ 'core/image' ], [ 'core/image' ] ] }
			/>
		);
	},
} );

Great job 👍👍👌:shipit:

lastSelectedIndex,
isLocked,
canDuplicate,
} = props;

This comment has been minimized.

Copy link
@noisysocks

noisysocks Aug 15, 2018

Member

Nit: you could put newlines into these function arguments if you don't want props in the scope.

withDispatch( ( dispatch, {
	clientIds,
	rootClientId,
	// etc.
} ) ) {
	// etc.
}
removeBlocks( clientIds );
if ( ! isLocked ) {
removeBlocks( clientIds );
}

This comment has been minimized.

Copy link
@noisysocks

noisysocks Aug 15, 2018

Member

Nice, a sneaky bug fix!

@talldan talldan merged commit a6ae71e into master Aug 15, 2018

2 checks passed

codecov/project 50.57% (-0.03%) compared to 8085f2e
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@talldan talldan deleted the add/insert-before-after-menu-items branch Aug 15, 2018

@mtias

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2018

Nice one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.