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

Drop external folders onto the Tree View to create new project folders #1209

Merged
merged 2 commits into from Dec 7, 2017

Conversation

Projects
None yet
3 participants
@50Wliu
Member

50Wliu commented Dec 6, 2017

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

Implements the popularly-requested feature #480. Basically if no existing folder is selected the Tree View adds the entries in the data transfer event as new project folders. There are a few rough edges with this implementation, such as but not limited to:

  • Folders must be dropped to a blank section of the Tree View after the last entry. This means that if the Tree View scrolls you will be unable to create a project folder as the entries will get added to the last project.
    • I would like to add support to be able to create project folders when dropping them between project folders, similar to how root drag-and-drop works currently.
  • Cannot drag and drop folders onto the Tree View when no Tree View exists.

Alternate Designs

No alternatives were considered.

Benefits

External folders can now be dragged-and-dropped into the Tree View to be added as project folders.

Possible Drawbacks

I don't believe there are any drawbacks with this other than the limitations addressed above.

Applicable Issues

Fixes #480

@@ -1056,6 +1056,9 @@ class TreeView
# Drop event from OS
for file in e.dataTransfer.files
@moveEntry(file.path, newDirectoryPath)
else if e.dataTransfer.files.length

This comment has been minimized.

@liuderchi

liuderchi Dec 7, 2017

Contributor

Would e.dataTransfer.files be undefined ?

@liuderchi

liuderchi Dec 7, 2017

Contributor

Would e.dataTransfer.files be undefined ?

This comment has been minimized.

@50Wliu

50Wliu Dec 7, 2017

Member

Good question. files is a standard DataTransfer property that will always be available. https://developer.mozilla.org/en-US/docs/Web/API/DataTransfer/files

@50Wliu

50Wliu Dec 7, 2017

Member

Good question. files is a standard DataTransfer property that will always be available. https://developer.mozilla.org/en-US/docs/Web/API/DataTransfer/files

This comment has been minimized.

@liuderchi

liuderchi Dec 7, 2017

Contributor

Thanks for the info. BTW it says that dataTransfer exists on all drag events. Good News.

@liuderchi

liuderchi Dec 7, 2017

Contributor

Thanks for the info. BTW it says that dataTransfer exists on all drag events. Good News.

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Dec 7, 2017

Member

I looked into implementing drag-and-drop indicators. It will be harder than I thought because then you'll have to differentiate between moving external folders into existing projects or whether to create a new project folder for it. I'll punt that for a later PR.

Member

50Wliu commented Dec 7, 2017

I looked into implementing drag-and-drop indicators. It will be harder than I thought because then you'll have to differentiate between moving external folders into existing projects or whether to create a new project folder for it. I'll punt that for a later PR.

@50Wliu 50Wliu merged commit 10a979c into master Dec 7, 2017

1 check was pending

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details

@50Wliu 50Wliu deleted the wl-external-drop branch Dec 7, 2017

@here

This comment has been minimized.

Show comment
Hide comment
@here

here Mar 7, 2018

Note that as of 2018-03-06 , this request remains unreleased in the Atom core editor with the tree-view core package at v221.3 or v222 based on https://github.com/atom/tree-view/releases

here commented Mar 7, 2018

Note that as of 2018-03-06 , this request remains unreleased in the Atom core editor with the tree-view core package at v221.3 or v222 based on https://github.com/atom/tree-view/releases

@liuderchi

This comment has been minimized.

Show comment
Hide comment
@liuderchi

liuderchi Mar 7, 2018

Contributor

Share a shell snippet for checking releases

# atom latest releases and corresponding tree-view version
$ curl https://api.github.com/repos/atom/atom/releases/latest -s | grep 'tag_name'

  "tag_name": "v1.24.0",

$ curl https://raw.githubusercontent.com/atom/atom/v1.24.0/package.json -s | grep 'tree-view'

    "tree-view": "0.221.3",
Contributor

liuderchi commented Mar 7, 2018

Share a shell snippet for checking releases

# atom latest releases and corresponding tree-view version
$ curl https://api.github.com/repos/atom/atom/releases/latest -s | grep 'tag_name'

  "tag_name": "v1.24.0",

$ curl https://raw.githubusercontent.com/atom/atom/v1.24.0/package.json -s | grep 'tree-view'

    "tree-view": "0.221.3",
@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Mar 7, 2018

Member

@here we're dealing with tree-view spec failures that need to be fixed before we feel comfortable updating it in Atom itself. Sorry for the delay.

Member

50Wliu commented Mar 7, 2018

@here we're dealing with tree-view spec failures that need to be fixed before we feel comfortable updating it in Atom itself. Sorry for the delay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment