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

Fix drop issue #1725

Closed
wants to merge 1 commit into from
Closed

Fix drop issue #1725

wants to merge 1 commit into from

Conversation

laysent
Copy link
Contributor

@laysent laysent commented Apr 8, 2018

Summary

I have created a demo to illustrate this issue, feel free to look at here

If you look at src/component/base/DraftEditor.react.js, there is a problem with _dragCount:

When you drag a file into editor and drop it, onDragEnter will be triggered to increase this._dragCount. However, onDragLeave will not be triggered when dropping, thus this._dragCount will not be decreased back to 0.

Thus, after dragging and dropping, when user drags another file and NOT drop it, both onDragEnter and onDragLeave will be triggered. This time, this._dragCount will be increased then decreased. However, since it's initial value after dropping is not 0, it will not be decreased back to 0 after dragLeave, thus it will not trigger this.exitCurrentMode() in onDragLeave. Editor remains in "drag" mode.

To resolve this issue, simply set _dragCount back to 0 when dropping the file.

Test Plan

I don't think one is necessary for this issue. But would be happy to provide one if required.

The problem:
Previously when file is dropped,
`_dragCount` will not be cleared and remains positive.
Next time when drag enter and leave,
it won't change back to "edit" mode,
as `_dragCount` is not 0 but still positive.
@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 cla@fb.com. 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!

@laysent
Copy link
Contributor Author

laysent commented Aug 16, 2018

@flarnie Hi, Flarnie, may I know if we could merge this fix? If you have any question about the fix or repro steps, feel free to let me know.

@niveditc
Copy link
Contributor

niveditc commented Oct 5, 2018

@laysent - thanks for submitting this PR! It looks safe to merge as far as I can tell. Could you possibly include a video of the issue & fix in the test plan? Basically, this would just be a screen recording in the demo that you linked to, making it easier to quickly understand the issue & fix :)

@laysent
Copy link
Contributor Author

laysent commented Oct 6, 2018

@niveditc Thanks for the feedback.
Following is a screen record of the demo in this repo: https://github.com/laysent/draft-js-drag-and-drop-issue
issue
The main problem is, when dropped a file first and not dropping a file second time, it will not come back to "edit" mode and thus will not handle enter correctly anymore.
Following is the code related to draft.js for this demo:

<Editor
  editorState={this.state.editorState}
  onChange={this.onChange /* to update this.state.editorState */}
  handleReturn={() => 'handled'}
  handleDroppedFiles={() => 'handled'}
/>     

I have simplified some code above and only kept the necessary for illustration. All code can be found in this demo repo: https://github.com/laysent/draft-js-drag-and-drop-issue/blob/master/src/App.js

Copy link
Contributor

@niveditc niveditc left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@niveditc
Copy link
Contributor

niveditc commented Oct 7, 2018

Next steps are that:

  • I'll import this and run FB's test stack.
  • Do a few sanity checks in the browser.
  • Provided the steps above are successful I'll merge it in tomorrow (Monday). In the slim chance that something breaks it's better to do a risky merge on a weekday than a weekend.

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.

niveditc has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@laysent
Copy link
Contributor Author

laysent commented Oct 8, 2018

@niveditc Sounds great. Let me know if there is anything further I could help :-)

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

I have created a demo to illustrate this issue, feel free to look at [here](https://github.com/laysent/draft-js-drag-and-drop-issue)

If you look at `src/component/base/DraftEditor.react.js`, there is a problem with `_dragCount`:

When you drag a file into editor and drop it, `onDragEnter` will be triggered to increase `this._dragCount`. However, `onDragLeave` will not be triggered when dropping, thus `this._dragCount` will not be decreased back to 0.

Thus, after dragging and dropping, when user drags another file and NOT drop it, both `onDragEnter` and `onDragLeave` will be triggered. This time, `this._dragCount` will be increased then decreased. However, since it's initial value after dropping is not 0, it will not be decreased back to 0 after `dragLeave`, thus it will not trigger `this.exitCurrentMode()` in `onDragLeave`. Editor remains in "drag" mode.

To resolve this issue, simply set `_dragCount` back to 0 when dropping the file.

**Test Plan**

I don't think one is necessary for this issue. But would be happy to provide one if required.
Pull Request resolved: facebookarchive#1725

Differential Revision: D10234644

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

I have created a demo to illustrate this issue, feel free to look at [here](https://github.com/laysent/draft-js-drag-and-drop-issue)

If you look at `src/component/base/DraftEditor.react.js`, there is a problem with `_dragCount`:

When you drag a file into editor and drop it, `onDragEnter` will be triggered to increase `this._dragCount`. However, `onDragLeave` will not be triggered when dropping, thus `this._dragCount` will not be decreased back to 0.

Thus, after dragging and dropping, when user drags another file and NOT drop it, both `onDragEnter` and `onDragLeave` will be triggered. This time, `this._dragCount` will be increased then decreased. However, since it's initial value after dropping is not 0, it will not be decreased back to 0 after `dragLeave`, thus it will not trigger `this.exitCurrentMode()` in `onDragLeave`. Editor remains in "drag" mode.

To resolve this issue, simply set `_dragCount` back to 0 when dropping the file.

**Test Plan**

I don't think one is necessary for this issue. But would be happy to provide one if required.
Pull Request resolved: facebookarchive/draft-js#1725

Differential Revision: D10234644

fbshipit-source-id: 6b8ffeef21899d80f623db3554dd688038ed92ac
aforismesen added a commit to aforismesen/draft-js that referenced this pull request Jul 12, 2024
Summary:
**Summary**

I have created a demo to illustrate this issue, feel free to look at [here](https://github.com/laysent/draft-js-drag-and-drop-issue)

If you look at `src/component/base/DraftEditor.react.js`, there is a problem with `_dragCount`:

When you drag a file into editor and drop it, `onDragEnter` will be triggered to increase `this._dragCount`. However, `onDragLeave` will not be triggered when dropping, thus `this._dragCount` will not be decreased back to 0.

Thus, after dragging and dropping, when user drags another file and NOT drop it, both `onDragEnter` and `onDragLeave` will be triggered. This time, `this._dragCount` will be increased then decreased. However, since it's initial value after dropping is not 0, it will not be decreased back to 0 after `dragLeave`, thus it will not trigger `this.exitCurrentMode()` in `onDragLeave`. Editor remains in "drag" mode.

To resolve this issue, simply set `_dragCount` back to 0 when dropping the file.

**Test Plan**

I don't think one is necessary for this issue. But would be happy to provide one if required.
Pull Request resolved: facebookarchive/draft-js#1725

Differential Revision: D10234644

fbshipit-source-id: 6b8ffeef21899d80f623db3554dd688038ed92ac
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.

3 participants