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

Exclude focused block from block completer options #7273

Merged
merged 2 commits into from Jun 12, 2018

Conversation

Projects
None yet
2 participants
@brandonpayton
Member

brandonpayton commented Jun 11, 2018

Description

Today, the block completer offers a Paragraph option when invoked from a Paragraph block, offering to replace the current block with another of the same type. It is odd to see and takes a menu slot that could be filled with a useful option. This PR updates the block completer to exclude the current block type from its list of options.

How has this been tested?

I added a unit test and ran the unit tests successfully. I also manually tested the block completer, observed the absence of the Paragraph option when using the completer from a core/paragraph block (the only place it is used today), and successfully used the completer to insert a Quote block.

Screenshots

Before

screen shot 2018-06-11 at 1 17 08 pm

After

screen shot 2018-06-11 at 1 40 31 pm

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
Remove redundant option from block completer
Prior to this change, the block completer offered an option to replace
the current block with a Paragraph block even though the current block
was already a Paragraph block. This updates the block completer to
exclude the current block type from its list of options.

@brandonpayton brandonpayton self-assigned this Jun 11, 2018

@brandonpayton brandonpayton requested a review from noisysocks Jun 11, 2018

@gziolo

gziolo approved these changes Jun 12, 2018

It works as expected, small but nice improvement. I had one comment where the current function name suggests that it would return an object, I proposed a different name.

*
* @return {string} The UID of the parent where a newly inserted block would be placed.
*/
function defaultGetBlockInsertionParent() {

This comment has been minimized.

@gziolo

gziolo Jun 12, 2018

Member

Can we name it defaultGetBlockInsertionParentUID to make it easier to follow?

@gziolo gziolo added this to the 3.1 milestone Jun 12, 2018

@brandonpayton brandonpayton merged commit 87f7420 into master Jun 12, 2018

2 checks passed

codecov/project 46.72% (+<.01%) compared to 3504ae7
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@brandonpayton

This comment has been minimized.

Member

brandonpayton commented Jun 12, 2018

Thanks for the review, @gziolo! Good point about the function name. I changed it before merging.

@brandonpayton brandonpayton deleted the fix/redundant-block-completer-option branch Jun 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment