Writing Flow: fix list selection

[ellatrix]

Description

Fixes #12656.

The problem is that we're looking at a collapsed range when determining if a range is at the vertical edge of an element.

Added an e2e test.

How has this been tested?

Screenshots

Types of changes

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR. .

[aduth]

The problem is that we're looking at a collapsed range when determining if a range is at the vertical edge of an element.

It's not immediately clear to me: Why does this make a difference? Or, to refresh my memory, why do we collapse the selection at all in this function?

[aduth]

I sorta wondered if this might have had something to do with an issue I've experienced in Chrome when dealing with selections that occur at ends of lines, where the dimensions are (unexpectedly) reported to exist on the following (preceding?) line:

https://codepen.io/aduth/pen/WPjQoW

This can happen a lot in our implementation when trying to ArrowDown after clicking at the end of a line of text. If the line is the second-to-last in a paragraph, it can often skip to the next block unexpectedly.

But I tested the original issue in Firefox (where this bug/inconsistency does not exist) and it works correctly ¯\_(ツ)_/¯

[ellatrix]

We seem to collapse it for this calculation:

https://github.com/WordPress/gutenberg/pull/19721/files#diff-09079d098673dd8130aa10e0f430a939R115-R123

And also for the horizontal edge calculation, in which case we want the left/right edge of the collapsed range, not the range as a whole (which can change if multiple line are selected).

[ellatrix]

@aduth That's correct. I'm aware of the Chrome issue where it selection an invisible bit of the side of the first line. I'm unsure how to account for this. When taking the rectangle of the original range, the rectangle seems to be correct, so that's why I used it instead.

[ellatrix]

I made an error when writing the e2e test and now it works fine.

[aduth]

There was a failing test. Not one of the usual failures, but doesn't seem like it should be related:

FAIL packages/e2e-tests/specs/editor/plugins/inner-blocks-allowed-blocks.test.js (12.198s)
  Allowed Blocks Setting on InnerBlocks 
    ✓ allows all blocks if the allowed blocks setting was not set (4083ms)
    ✓ allows the blocks if the allowed blocks setting was set (3072ms)
    ✕ correctly applies dynamic allowed blocks restrictions (3413ms)
  ● Allowed Blocks Setting on InnerBlocks  › correctly applies dynamic allowed blocks restrictions
    expect(received).toEqual(expected)
    Difference:
    - Expected
    + Received
      Array [
    -   "Gallery",
    -   "Video",
    +   "Image",
    +   "List",
      ]
      81 | 		expect(
      82 | 			await getAllBlockInserterItemTitles()
    > 83 | 		).toEqual( [
         | 		  ^
      84 | 			'Gallery',
      85 | 			'Video',
      86 | 		] );
      at Object.toEqual (specs/editor/plugins/inner-blocks-allowed-blocks.test.js:83:5)
          at runMicrotasks (<anonymous>)

I've restarted the build.

[aduth]

Tests well to me 👍

[ellatrix]

Thanks for the review!