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

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 added [Type] Bug An existing feature does not function as intended [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Type] Performance Related to performance efforts [Block] List Affects the List Block labels Dec 26, 2019
@youknowriad youknowriad self-assigned this Dec 26, 2019
@youknowriad youknowriad merged commit 60fb9a0 into master Dec 26, 2019
@youknowriad youknowriad deleted the fix/list-block-shortcuts branch December 26, 2019 14:30
@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
Labels
[Block] List Affects the List Block [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Type] Bug An existing feature does not function as intended [Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant