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 error triggering after duplicating a block making it unselectable. #38760

Merged
merged 2 commits into from
Feb 14, 2022

Conversation

delowardev
Copy link
Contributor

@delowardev delowardev commented Feb 12, 2022

Fixes: #38759

Description

After duplicating a block from List View, it doesn't select that block and throws an error.

Testing Instructions

Screenshots

Types of changes

Bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).
  • I've updated related schemas if appropriate.

@delowardev delowardev requested a review from ellatrix as a code owner February 12, 2022 13:21
@skorasaurus skorasaurus added the [Feature] List View Menu item in the top toolbar to select blocks from a list of links. label Feb 12, 2022
@talldan
Copy link
Contributor

talldan commented Feb 14, 2022

Thanks for looking into this issue @delowardev.

The more I look at the code in trunk, the more it looks broken, and I'm also not sure what the right way to fix it is.

As you mention, selectEditorBlocks is sometimes called with clientIds as a param, sometimes with an event as a param. I'm also not sure what the original intention of the call to select blocks was in the block settings dropdown. Perhaps to prevent a focus loss?

I'm going to add some reviewers that have worked on this in the past for some further opinions. If I recall correctly @adamziel and @noisysocks worked on the BlockSettingsDropdown in List View.

@noisysocks
Copy link
Member

I'm also not sure what the original intention of the call to select blocks was in the block settings dropdown. Perhaps to prevent a focus loss?

I'm not 100% sure either. This bit of the codebase has had a lot of iteration so very possible it's not used anymore. Feel free to remove it and see what happens 😀 just be sure to test List View in the site editor, post editor, widgets editor, navigation block toolbar, and experimental navigation editor.

@talldan
Copy link
Contributor

talldan commented Feb 14, 2022

After looking at the code for a while, I think I have a good idea for a fix.

It looks like all that needs to be changed is this line:

__experimentalSelectBlock={ selectEditorBlock }

Which can be changed to this:

__experimentalSelectBlock={ selectBlock }

Reasoning: selectBlock already accepts a clientId as the first param and doesn't expect an event, so seems to match the code in BlockSettingsDropdown. I can't be sure, but I think this is probably what was intended, but someone made a typo and accidentally typed selectEditorBlock.

@delowardev delowardev requested a review from talldan February 14, 2022 07:30
Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

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

That seems to work well now, thanks for spotting the bug and fixing it.

@talldan talldan merged commit eb12c4b into WordPress:trunk Feb 14, 2022
@github-actions github-actions bot added this to the Gutenberg 12.7 milestone Feb 14, 2022
@cbravobernal cbravobernal changed the title Fix: Select block after duplicate Fix error triggering after duplicating a block making it unselectable. Feb 26, 2022
@cbravobernal cbravobernal added the [Type] Bug An existing feature does not function as intended label Feb 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] List View Menu item in the top toolbar to select blocks from a list of links. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'Duplicate' blocks from "List View" throws an error.
5 participants