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

[REVIEW ONLY] Project manager revamp #8885

Closed
wants to merge 14 commits into from

Conversation

dangoor
Copy link
Contributor

@dangoor dangoor commented Aug 27, 2014

Note that this pull request is based on jeff/splitview-1x2. I'll make a new PR once that is merged into master.

cc @ingorichter

@dangoor
Copy link
Contributor Author

dangoor commented Aug 29, 2014

This is an initial PR for #8788

@dangoor
Copy link
Contributor Author

dangoor commented Aug 29, 2014

Here are two examples I have of the new extension APIs in use:

var EVENT_CHANGE = "change";

/**
* Determine if an entry from the treeData map is a directory.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the naming and comment are a bit confusing. We are checking for a directory, but the method is called isFile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, good catch! I've changed the comment.

@dangoor
Copy link
Contributor Author

dangoor commented Sep 3, 2014

I've addressed the review comments so far. As soon as splitview lands on master, I will rebase on top of master and make a new pull request targeting master.

@dangoor
Copy link
Contributor Author

dangoor commented Sep 3, 2014

Here's my current list of issues/things to test that I'm looking into:

  • Failing ProjectManager tests
  • Switching project not updating the tree?
    • when opening brackets-git (which has a src folder) src is not getting updated. A merge algorithm problem?
  • Selected directory is missing trailing slash in ProjectModel._selections
  • brackets-git won't refresh on filesystem changes
    • need to ensure that when an item has to refresh its object is replaced
  • Test project base URL functionality
  • Right clicking on another node while the context menu is up does not move the context to that other node
  • Clear TODOs in new code
  • Rather than selectIfVisible the old code looks at whether the current doc has become visible. curDoc = DocumentManager.getCurrentDocument(); and then if (_hasFileSelectionFocus() && curDoc && data)
  • Error cases for create and rename
  • Creation errors are not visible (that window timeout thing)
  • Check for invalid filenames on rename
  • The double click to rename shouldn't take effect if it's been too long since the first click
  • Control-double click toggles siblings
  • Alt-double click is supposed to collapse subtree but does not seem to do anything for me.
  • Improve _fileSystemChange handler?
  • ProjectManager.showInTree needs to expand nodes and not do anything if the path is outside the project or doesn't exist
  • ProjectManager.refreshFileTree needs to make sure it doesn't run multiple times and has the 1 second delay between runs if there's one pending. Also double check that it's returning a promise that's resolved when it's done.
  • Run all tests
  • Positioning is wrong when node is scrolled into view (too far left)
  • Verify ProjectManager events
    • projectRefresh doesn't seem likely to be fired!
  • Can ViewUtils.getFileEntryDisplay go away?
  • Close dropdowns on scroll (Menus.closeAll())
  • ProjectManager._fileSystemRename may not be used currently, but should be.
  • Make the actionCreator private in ProjectManager
  • Make treeData private in FTVM
  • Delete jstree
  • The old JSTree code would not select a directory if you click the disclosure triangle, but would select it if you click on the name. Do we care about that? That was probably because there was no differentiation between context and selection.
  • Find In not working on directory [Project Manager Revamp] Search in Folder doesn't work #8944

/**
* Returns false for files and directories that are not commonly useful to display.
*
* @param {FileSystemEntry} entry File or directory to filter
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we should use {!FileSystemEntry} to indicate that the parameter can not be null.

*
* @param {string} path path to create
* @param {boolean} isFolder true if the new entry is a folder
* @return {Promise} resolved when the file or directory has been created.
Copy link
Contributor

Choose a reason for hiding this comment

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

for consistency we should either use {jQuery.Promise} or {Promise} all over the place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I switched to {jQuery.Promise} since that's what we're using now.

…s-git.

brackets-git needs to apply class changes to the tree *after* running git status.
Since that happens asynchronously, brackets-git needs some way to signal that
a change has occurred. The solution here is a new "forceRefresh" that causes the
entire tree to re-render. (Not optimally efficient, but React still minimizes actual
DOM changes.)
@dangoor dangoor mentioned this pull request Sep 8, 2014
30 tasks
@dangoor
Copy link
Contributor Author

dangoor commented Sep 9, 2014

Closing this in favor of #9015, because I've addressed the review comments here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants