Skip to content
This repository was archived by the owner on Feb 6, 2023. It is now read-only.

Correct warning condition #2049

Closed
wants to merge 1 commit into from
Closed

Correct warning condition #2049

wants to merge 1 commit into from

Conversation

pindlebot
Copy link

@pindlebot pindlebot commented Mar 30, 2019

Summary

To my knowledge, warning works the way invariant does; it warns if the first argument is false not true.

Prior to this PR, moveSelectionBackward warns when the selection is collapsed when it should do the opposite.

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@claudiopro
Copy link
Contributor

Hey @unshift, welcome to Draft.js and thanks for spending the time to submit a PR!

You're absolutely right, the warning function in fbjs prints a warning if the condition is false:

https://github.com/facebook/fbjs/blob/11f270b715ec7b39cba6a6ba8a72dcc3d0462e57/packages/fbjs/src/__forks__/warning.js#L44

Therefore, in order to warn when the selection is not collapsed, the first argument must be selection.isCollapsed().

@claudiopro
Copy link
Contributor

I reproduced the problem in the examples. This is good to merge.
Screen Shot 2019-03-31 at 12 20 00 PM

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@claudiopro is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link

This pull request has been merged in ffd8f59.

@fabiomcosta
Copy link
Contributor

Good to know those were incorrect! Thank you!

jdecked pushed a commit to twitter-forks/draft-js that referenced this pull request Oct 9, 2019
Summary:
**Summary**

To my knowledge, ```warning``` works the way ```invariant``` does; it warns if the first argument is false not true.

Prior to this PR, ```moveSelectionBackward``` warns when the selection is collapsed when it should do the opposite.
Pull Request resolved: facebookarchive#2049

Differential Revision: D14699712

fbshipit-source-id: bd649fa6fdc60346d80be27ab4217324c3ef21d6
mmissey pushed a commit to mmissey/draft-js that referenced this pull request Mar 24, 2020
Summary:
**Summary**

To my knowledge, ```warning``` works the way ```invariant``` does; it warns if the first argument is false not true.

Prior to this PR, ```moveSelectionBackward``` warns when the selection is collapsed when it should do the opposite.
Pull Request resolved: facebookarchive#2049

Differential Revision: D14699712

fbshipit-source-id: bd649fa6fdc60346d80be27ab4217324c3ef21d6
alicayan008 pushed a commit to alicayan008/draft-js that referenced this pull request Jul 4, 2023
Summary:
**Summary**

To my knowledge, ```warning``` works the way ```invariant``` does; it warns if the first argument is false not true.

Prior to this PR, ```moveSelectionBackward``` warns when the selection is collapsed when it should do the opposite.
Pull Request resolved: facebookarchive/draft-js#2049

Differential Revision: D14699712

fbshipit-source-id: bd649fa6fdc60346d80be27ab4217324c3ef21d6
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants