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

Restrict drag-and-drop actions to events from the Tree View #1293

Merged
merged 6 commits into from Apr 29, 2019

Conversation

Projects
None yet
2 participants
@50Wliu
Copy link
Member

commented Dec 20, 2018

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

Previously, the Tree View would respond to all items dragged over it, irrespective of whether or not any action could be performed with said item. This would result in the illusion of being able to drop anything onto the Tree View, whereas in reality the drop action wouldn't do anything.

With this PR, the Tree View only responds when it knows that it can handle the event.

Before After
tree-view-drag-and-drop-before tree-view-drag-and-drop-after

(Note: you'll notice that an empty pane is still created. Fixed in atom/tabs#550)

Alternate Designs

None.

Benefits

There are two benefits.

  1. Tree View folders will not be incorrectly highlighted when something that can't be dropped onto the Tree View is dragged over it.
  2. Tree View does not unnecessarily prevent drag-and-drop event propagation, meaning that other packages (such as Tabs) can continue to receive those events.

Possible Drawbacks

In the unlikely event that someone was using the atom-tree-view-event or initialPaths data items in a different package to work with the Tree View, that functionality will break. They will now need to use atom-tree-view-root-event and atom-tree-view-event (in addition to initialPaths), respectively.

Applicable Issues

atom/tabs#539, which prior to this PR was not working correctly when a tab was dragged over the Tree View as it was preventing any drag-and-drop events from propagating up to the tabs package.

50Wliu added some commits Dec 20, 2018

@rafeca

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2019

Thanks a lot!

Looks good to me, once you resolve the conflicts we can merge it 😃

@50Wliu

This comment has been minimized.

Copy link
Member Author

commented Apr 29, 2019

once you resolve the conflicts we can merge it

The consequences of creating multiple PRs that all add onto the end of the drag-and-drop specs 😬 😅

@50Wliu 50Wliu merged commit 9329d0c into master Apr 29, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@50Wliu 50Wliu deleted the wl-restrict-drag-and-drop branch Apr 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.