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 the multiple list block shortcuts #19334

Merged
merged 1 commit into from Dec 26, 2019
Merged

Conversation

@youknowriad
Copy link
Contributor

youknowriad commented Dec 26, 2019

I noticed two things:

  • List indent/outdent shortcuts don't work properly when you have multiple list blocks (callbacks are triggered in all list blocks while they should only affect the selected blocks)
  • Also this has some impact on performance since keydown handlers number grow significantly as we add list blocks to the post.

This PR wraps those shortcuts in an isSelected check to fix that.

That said, I also took a look at the RichtextShortcut component and after some thinking I concluded that this component is useless and also misleading.

  • I wonder whether I should wrap the component automatically in ifBlockEditSelected like block controls... but that's not a good approach because conceptually speaking, you can have a RichText outside a block or multiple RichText in the same block, so this won't solve anything really.
  • It gives you the impression that it tied to RichText somehow but in reality, it's not it's just a proxy to the KeyboardShortcuts component.
  • I think ideally, we should just deprecate that component. I didn't do that yet.
@youknowriad youknowriad self-assigned this Dec 26, 2019
@youknowriad youknowriad requested a review from ellatrix Dec 26, 2019
@youknowriad youknowriad merged commit 60fb9a0 into master Dec 26, 2019
2 checks passed
2 checks passed
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@youknowriad youknowriad deleted the fix/list-block-shortcuts branch Dec 26, 2019
@youknowriad youknowriad added this to the Gutenberg 7.2 milestone Jan 6, 2020
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.