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

Stop creating bookmarks when touching blocks that the selection is not in #38

Closed
caffodian opened this issue Oct 28, 2016 · 5 comments
Closed
Assignees

Comments

@caffodian
Copy link

I was talking through this problem and then realized this is horrible to do. This must be leftover cruft from the initial "touch the whole document" version.

@caffodian caffodian self-assigned this Oct 28, 2016
@caffodian
Copy link
Author

Articulating some points better:

Doing one bookmark per batch is nice. However, restoring bookmarks is expensive regardless of whether it actually does something visible to the user (because span removal is required, or checks are required.)

Also, by creating the bookmark only once per batch, we create problems when you have multiple excessively large blocks. Because things are now more responsive, it's possible to do more things and get the selection restored to the wrong place.

@caffodian
Copy link
Author

We need to:

  • not create bookmarks unless needed
  • also shorten the lifetime of any other bookmark, as much as possible. Ideally it only gets created when we actually are working on a range that crosses it.

@caffodian
Copy link
Author

I might be going insane but it's possible that with all the various changes we made along the way, that bookmarking is no longer needed (except for span removal)

@caffodian
Copy link
Author

One ugly case we don't handle properly (which makes figuring out bookmarks difficult) - if a word is actually multiple text nodes and you type into the middle of it

@caffodian
Copy link
Author

The render event already takes a rootElement. Is it sufficient to determine if the selection falls inside?

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

No branches or pull requests

1 participant