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

Fix selection error when backwards selections cover fields #410

Merged
merged 2 commits into from
Jun 17, 2024

Conversation

jonathonherbert
Copy link
Contributor

What does this change?

Fixes an error where backwards selections that extend beyond a field are passed into the field view, causing range errors.

The errors are largely harmless – they're a no-op on the state and do not halt the outer editor – but they're causing some noise in our downstream CMS's error reporting.

There's a question as to whether fields should be aware of selections that cover them entirely. I don't think we've seen a use case yet, but we could map and clamp the selection values, rather than just ignoring, if we wanted that behaviour.

How to test

  • The new unit tests should pass. The commit history shows the failure, then the pass.
  • Try adding a backwards selection across an element. You should not see red ink in the console:

error-selection

@jonathonherbert jonathonherbert requested a review from a team as a code owner June 10, 2024 10:15
Base automatically changed from rm/noting-extension-bug to main June 10, 2024 16:04
@rhystmills
Copy link
Contributor

Think this may need a rebase now but I am happy to review as soon as possible.

@jonathonherbert
Copy link
Contributor Author

Thanks @rhystmills, just done that. (Now we no longer use semantic-release in this library, perhaps it's worth turning off squash and merge, which IIUC makes conflicts more likely.)

Copy link
Contributor

@rhystmills rhystmills left a comment

Choose a reason for hiding this comment

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

Makes sense as a fix, and all tests passing 👍

@jonathonherbert jonathonherbert merged commit 1dc4261 into main Jun 17, 2024
3 checks passed
@jonathonherbert jonathonherbert deleted the jsh/fix-selection-error branch June 17, 2024 08:27
@jonathonherbert jonathonherbert mentioned this pull request Jun 17, 2024
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.

None yet

2 participants