-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Conversation
@andrewbranch - thanks for putting up this fix. I won't get a chance to review until next weekend, but if you could rebase on master in the meanwhile, that would be a big help! |
@niveditc done 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
I checked the two current usages of hasEdgeWithin
in the Draft core & both of them remain unchanged with this fix, so I think this should be an overall improvement since the logic is correct now :)
Next steps are that:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
niveditc has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Fixes facebookarchive#1700, which includes a detailed description and reproduction of the issue. Hopefully it's ok that I refactored the true/false assertions not to use snapshots; I found them confusing and tedious and figured it was probably just an artifact of migrating to Jest a long time ago. Pull Request resolved: facebookarchive#1811 Differential Revision: D10234643 fbshipit-source-id: 531e45c8fe1708fb440240e1a7bd121cb126cd08
Summary: Fixes #1700, which includes a detailed description and reproduction of the issue. Hopefully it's ok that I refactored the true/false assertions not to use snapshots; I found them confusing and tedious and figured it was probably just an artifact of migrating to Jest a long time ago. Pull Request resolved: facebookarchive/draft-js#1811 Differential Revision: D10234643 fbshipit-source-id: 531e45c8fe1708fb440240e1a7bd121cb126cd08
Summary: Fixes #1700, which includes a detailed description and reproduction of the issue. Hopefully it's ok that I refactored the true/false assertions not to use snapshots; I found them confusing and tedious and figured it was probably just an artifact of migrating to Jest a long time ago. Pull Request resolved: facebookarchive/draft-js#1811 Differential Revision: D10234643 fbshipit-source-id: 531e45c8fe1708fb440240e1a7bd121cb126cd08
Fixes #1700, which includes a detailed description and reproduction of the issue.
Hopefully it's ok that I refactored the true/false assertions not to use snapshots; I found them confusing and tedious and figured it was probably just an artifact of migrating to Jest a long time ago.