Skip to content

Conversation

@VolgaIgor
Copy link
Contributor

Hello!
I have noticed a problem that when using a nested editor in a block, the parent editor stops correctly identifying the current block when hovering over or clicking on the nested editor block.

Example:
unnormal

You can see that the toolbox menu jumps when you hover or click on the nested editor. This happened because in hover/click events, all editors in the chain received the block of the lower-level editor in which the event occurred. So the top-level editors could not correctly determine in which block of their instance the event occurred.

I fixed this bug by making a function that determines the block belonging to a current editor instance when an event occurs for each editor in the nesting chain.

Example after fix:
normal

  • The example used the Spoiler block, but the problem was relevant for any case of using a nested editor

Copy link
Member

@neSpecc neSpecc left a comment

Choose a reason for hiding this comment

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

Please, write few tests for this case

@VolgaIgor
Copy link
Contributor Author

Hello!
Sorry for the long waiting. I have added a test for this case.

@neSpecc

* And also allows for top level editors to identify in which block
* of their instance the event occurred in a nested editor block.
*/
private findByNodeBlockBelongsToCurrentInstance(node: Element): Block|null {
Copy link
Member

Choose a reason for hiding this comment

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

we have the getBlock() method of BlockManager which seems to be used for the case "find block by element".

Maybe we can:

  1. Improve that method to handle case with nested editor instance
  2. Reuse it here
  3. (I'm not sure) Move this method to /utils/blocks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is not the right solution to use this method. Because for now getBlock() method is used to get a block only by its holder element, without searching by children elements. So, searching for the parent editor block by the nested editor element will greatly change the logic of this method. And as I can tell from a quick look at the code, it will break some modules.

A better solution may be to combine this method with methods getBlockByChildNode() and/or setCurrentBlockByChildNode(). However, this also cannot be done clearly, because it is also will break some modules. For example, as far as I understand, in this case the paste module will start pasting data into all editors in the chain, rather than a instance in focus: paste.ts. Also, selection processing methods wait to receive blocks of the editor in which the selection occurs, and not of a nested one.

So, to combine this method with others in the current implementation, it will be necessary to add a flag option that would prohibit searching in nested editors, because many methods don't need it. Therefore, in my commit I did not combine them for now, and made a separate method that is used only in places where it is necessary.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants