Add property "isEmpty"

[SamyPesse]

Fix #862 by adding a property isEmpty. The computation tries to be performant by not computing state.fragment when the selection is "simple".

I've also updated the "Hovering menu", it works perfectly with it.

@ianstormtaylor I'm not sure where to put unit tests for this property.

[Soreine]

After checking that the selection is not collapsed, you can probably check if at least one of startOffset or endOffset equals 0. It's a cheap check, but a condition for the selection to be empty. It could avoid computing a fragment in most cases.

[Soreine]

And continuing on this idea, you can afterward check if the other offset is at the end of its node.

[ianstormtaylor]

Hey @SamyPesse and @Soreine I have a thought... this issue also comes up when people triple-click a block and then do something like setBlocks('header_block'), the trailing block also gets set to a header since the selection ends up inside the start of it.

So I'm wondering, do you think there's a way we can actually detect the double/triple-click scenario and instead tweak the selection to be what we expect it to? I feel like tweaking the selection itself might end up with covering more of the edge cases without as much complexity to keep track of elsewhere. I think Draft.js tweaks the selection, but I'm not sure how they do it.

If so, I think that would be a better solution for us to reduce complexity. What do you think?

[SamyPesse]

I'm not sure I understand the tripe-click case. But I'm wondering if we can simply improve the Selection.normalize(document: Document): Selection to detect when the selection is between two block (start at the end of one, and ends at the start of another block), in that case we can collapse the selection.

@ianstormtaylor Will it work ?

[Soreine]

What Ian suggests is that we patch the few cases that can lead to such selections in a unintended way (double click, triple click, maybe some other cases?). We would detect when they happen and make sure to fix the selection there, instead of normalizing the selection everytime or checking everywhere for a isEmpty property

Sounds simpler. As long as we have a way to detect these special cases.

[SamyPesse]

I disagree with your approach @Soreine.

I think it makes more sense to always prevent this case of selection (Fake collapsed selection) whatever it has been caused by an user action or a transform from the application.

Preventing this invalid selection in one spot (Selection.normalize) will not have an impact on performances and will improve the reliability compared to trying to handle N potential edge cases.

We'll not need the selection.isEmpty property, since it'll only be checked in Selection.normalize or it will never differ from selection.isCollapsed.

[ianstormtaylor]

Note, ignore everything between the horizontal rules in the end, I think after writing it all out I agree that going with isEmpty might be cleaner. At least for now.

---

My comment was what @Soreine mentioned, but I think both approaches have the potential to be viable, it depends on a few things...

Detect Double/Triple-click

With the approach @Soreine described, we'd try to somehow detect that we were double or triple-clicking and then adjust the selection, since we know that in the cases we've seen the user doesn't actually want the selection to extend to the adjacent nodes.

This solution is nice because it seems like it most closely matches user intent for these specifics cases where the problem is, which will lead to reduced confusion.

The potential downside is that I'm not sure if there is a way to detect it reliably, but I say that without having investigated at all so that would be the next step.

Normalize Fake-collapsed

This is the approach @SamyPesse described.

The problem with it, and why I didn't think to suggest it, is that there are certain situations where the "fake-collapsed" selection is actually valid, and normalizing them away would result in odd behavior. For example, if you shift + right arrow to extend the selection across a block, you pass through a point where you have a fake-collapsed selection.

Although, having said that, I think that might be the only time that pass through happens, so we could decide to special-case that instead potentially.

It seems like both might work? I think I'd rather go with the double/triple-click logic since it seems like it matches user intent closely, and eliminates us having to handle more edge cases with the core plugin transforms.

---

Actually, after writing out both of the downsides, it seems like either case is going to result in us having to police too many edge cases that might not be what the user intended.

So I think it would make sense to go with this PR and the isEmpty solution for now to solve the hovering menu case, and to tweak how getBlocksAtRange works to solve the setBlock case.

How does that sound?

Can you think of any problems that would result from getBlocksAtRange omitting any blocks that are "fake-collapsed" selected?

[SamyPesse]

@ianstormtaylor Yes it makes sense, I improved the PR with @Soreine's suggestion and it should be ready to be merged :)

[ianstormtaylor]

Thanks @SamyPesse!