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

Fix navigation menu insertion issues #18178

Merged
merged 2 commits into from Oct 31, 2019
Merged

Fix navigation menu insertion issues #18178

merged 2 commits into from Oct 31, 2019

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Oct 30, 2019

Description

On #18100 I was having some issues with insertion in the nav block:

  • the new __experimentalGetAllowedBlockTypes selector was throwing an error when blockListSettings didn't contain a setting for a particular block. Not entirely sure why this happens.
  • An undefined destinationRootClientId prop was being used to get the insertion index. This would mean the block would often be insterted in the wrong place.

The fixes:

  • Update the selector to be more like the hasInserterItems selector.
  • Use the already defined rootClientId prop.

How has this been tested?

  1. Add a navigation block
  2. Add menu items to the nav block
  3. Expect that the menu items are added correctly

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@talldan talldan self-assigned this Oct 30, 2019
@talldan talldan mentioned this pull request Oct 30, 2019
5 tasks
@talldan talldan changed the title Use rootClientId instead of destinationRootClientId Fix navigation menu insertion issues Oct 30, 2019
Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

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

This fixes inserting in the wrong position for the new "smart" appender. It is finally smart :O)

@draganescu
Copy link
Contributor

It's interesting that we've seen this after merge as in my local testing I wasn't getting any issues, visual or console.

@talldan talldan merged commit 6f172cd into master Oct 31, 2019
@talldan talldan deleted the fix/nav-block-insertion branch October 31, 2019 00:41
@youknowriad youknowriad added this to the Gutenberg 6.9 milestone Nov 11, 2019
CreativeDive pushed a commit to CreativeDive/gutenberg that referenced this pull request Nov 12, 2019
* Use rootClientId instead of destinationRootClientId

* Update __experimentalGetAllowedBlocks to use same check as `hasInserterItems`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants