Add tests for Editor.unhangRange() #4703
Conversation
🦋 Changeset detectedLatest commit: d28d619 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| export const output = { | ||
| anchor: { path: [0, 0], offset: 0 }, | ||
| focus: { path: [0, 0], offset: 25 }, | ||
| } |
There was a problem hiding this comment.
This doesn't look right: it completely skipped the second paragraph. I am going to fix this with a separate PR.
| export const output = { | ||
| anchor: { path: [0, 0], offset: 6 }, | ||
| focus: { path: [0, 2], offset: 0 }, | ||
| } |
There was a problem hiding this comment.
| <text> | ||
| before | ||
| <anchor /> | ||
| </text> | ||
| <text>selected</text> |
There was a problem hiding this comment.
In #3683 it was guessed that unhangRange should also unhang the starting point of the selection by moving it forward: #3683 (comment)
I wonder if introducing this behavior does make sense with the original intention of this method. Does it?
There was a problem hiding this comment.
I think the original intention was ambiguous here (afaik), so I'd be fine with introducing this change assuming no one strongly disagrees.
There was a problem hiding this comment.
This would solve at least this issue I think: #4793. Might also be good to consider inlines as well for this kind of case?
|
@dylans could someone please take a look at this PR? is there something wrong with it or was it just missed? Thanks! |
Just missed, reviewing again now, thanks! |
| export const output = { | ||
| anchor: { path: [0, 0], offset: 6 }, | ||
| focus: { path: [0, 2], offset: 0 }, | ||
| } |
| <text> | ||
| before | ||
| <anchor /> | ||
| </text> | ||
| <text>selected</text> |
There was a problem hiding this comment.
I think the original intention was ambiguous here (afaik), so I'd be fine with introducing this change assuming no one strongly disagrees.
| export const output = { | ||
| anchor: { path: [0, 0], offset: 0 }, | ||
| focus: { path: [0, 0], offset: 25 }, | ||
| } |
|
@e1himself this may have broken integration tests so I may need to revert it until all tests pass. I'll let you know. |
|
There aren't zero tests, as I discovered in #4203, but it's more tested accidentally. This is good to see; when I eventually get time to finish my PR I'll move the tests I added into this folder 👍 |
* Add tests for Editor.unhangRange() * Extend tests for `Editor.unhangRange()` to cover voids * Fix lint * Add changeset
* Add tests for Editor.unhangRange() * Extend tests for `Editor.unhangRange()` to cover voids * Fix lint * Add changeset
Description
Add tests for
Editor.unhangRange()Issue
Related to #3683
Example
n/a
Context
Editor.unhangRange()has zero tests. So it's really difficult to understand how it should work.Also any changes to its behavior will not have obvious effect visible in tests expectations.
This PR documents the existing behavior of
Editor.unhangRange()without modifying it.Checks
yarn test.yarn lint. (Fix errors withyarn fix.)yarn start.)yarn changeset add.)