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

Project panel bug fixes #845

Merged
merged 14 commits into from
May 10, 2012
Merged

Project panel bug fixes #845

merged 14 commits into from
May 10, 2012

Conversation

jasonsanjose
Copy link
Member

FYI @ryanstewart - Opportunistic refactor of sidebar resize handling to clean up dependencies in ProjectManager.js and brackers.js. Sets the stage for a future ProjectManager UI separation.

@ghost ghost assigned gruehle May 9, 2012
@gruehle
Copy link
Member

gruehle commented May 9, 2012

Reviewing

@@ -66,7 +66,7 @@ define(function (require, exports, module) {
// Navigate
exports.NAVIGATE_QUICK_OPEN = "navigate.quickOpen";
exports.NAVIGATE_GOTO_DEFINITION = "navigate.gotoDefinition";
exports.NAVIGATE_GOTO_LINE = "navigate.gotoLine";
exports.NAVIGATE_GOTO_LINE = "navigate.gotoLine";
Copy link
Member

Choose a reason for hiding this comment

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

Since you're fixing one, you might as well fix them all :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@gruehle
Copy link
Member

gruehle commented May 9, 2012

I found a bug in the new implementation:

  1. Open brackets/src folder (if not already open), and make sure all top-level folders are closed
  2. Open the editor folder
  3. Select Editor.js
  4. Close the editor folder (the editor folder itself should now be highlighted)
  5. Open the document folder

Results:
DocumentCommandHandlers.js is highlighted, but not selected.

@@ -0,0 +1,167 @@
/*
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for factoring this into a new file! :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Woohoo. And someday a refactored ProjectManager will fit in here too.

@gruehle
Copy link
Member

gruehle commented May 9, 2012

Done with initial review. Looks like you need to do another merge with master.

@jasonsanjose
Copy link
Member Author

Fixed the bug Glenn found. Merged with master.

@ryanstewart
Copy link
Contributor

Sweet, thanks for cleaning all this up @jason-sanjose. Looking forward to seeing it land.

@gruehle
Copy link
Member

gruehle commented May 10, 2012

Found another bug:

  1. Double click to close the sidebar
  2. Move the cursor near the left edge of the window until it turns into a resize cursor
  3. Try to drag the sidebar open. Problem 1: can't
  4. Realize that didn't work, so you try double-clicking. Problem 2: doesn't reopen
  5. Realize that didn't work, so you select "Show Sidebar". Problem 3: doesn't reopen
  6. Figuring "aw, what the heck", you try selecting "Show Sidebar" again. IT WORKS! :-)

@gruehle
Copy link
Member

gruehle commented May 10, 2012

I just noticed another issue too: the editor shadow is not adjusted when the window is resized. Open a file, scroll a bit so you get the shadow and make the window wider. The shadow doesn't get wider.

@jasonsanjose
Copy link
Member Author

Fixed the Glenn's latest issues.

@gruehle
Copy link
Member

gruehle commented May 10, 2012

Thanks, Jason. Everything is looking great, but there is one last minor issue: when the sidebar is collapsed, if you drag it slowly to open it, it always snaps shut. If you drag it really fast you can get it to open. It looks like this is due to the mousemove logic that snaps the sidebar closed when e.clientX < 10.

@gruehle
Copy link
Member

gruehle commented May 10, 2012

Looks great! Merging.

gruehle added a commit that referenced this pull request May 10, 2012
@gruehle gruehle merged commit ed38cf1 into master May 10, 2012
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