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

Conversation

@peterflynn
Copy link
Member

Fixes another part #2015 by maintaining panel max resize limits based on currently available slack space (i.e. space used by the editor area). This fixes part 1 of the 4-part list of issues there -- when dragging a panel to fill the whole UI, you can't overshoot and send the status bar (or other panels) sliding off the bottom of the window. And it gives us a clean foundation to build the remaining fixes on.

This also enables a cleaner fix to #3371 -- see pull #3931 which is based on this branch.

Here's what's changed:

  • PanelManager provides APIs to create panels below the editor area. JSLint
    and Find in Files now use these APIs. (Find in Files panel UI is now moved out of main-view.html into a template).
  • Move editor-holder ht calculations from EditorManager to PanelManager
  • EditorManager listens to PanelManager for resize notifications. PanelManager
    listens for window resize (moved from EditorManager) and for Resizer events
    on all bottom panels. The old EditorManager.resizeEditor() API is still used
    for edge cases such as status & search bar show/hide, extensions that add
    fixed top panels, and extensions that don't use Resizer APIs to hide/show
    panels.
  • Since PanelManager is listening, Resizer no longer pings EditorManager
    directly about show/hide/resize. (This would let us use Resizer as a generic splitter widget in other parts of the UI if needed).
  • Resizer emits events to notify PanelManager about any panels it may have missed (legacy extensions that add panels via
    Resizer instead of PanelManager).
  • Add max-size support to Resizer. Change Resizer min/max limits to operate in terms of panel outer height.
  • PanelManager updates all bottom panels' max sizes when anything is resized/shown/hidden.

- PanelManager provides APIs to create panels below the editor area. JSLint
and Find in Files now use these APIs.
- Move editor-holder ht calculations from EditorManager to PanelManager
- EditorManager listens to PanelManager for resize notifications. PanelManager
listens for window resize (moved from EditorManager) and for Resizer events
on all bottom panels. The old EditorManager.resizeEditor() API is still used
for edge cases such as status & search bar show/hide, extensions that add
fixed top panels, and extensions that don't use Resizer APIs to hide/show
panels.
- Since PanelManager is listening, Resizer no longer pings EditorManager
directly about show/hide/resize. Resizer emits events to notify PanelManager
about any panels it may have missed (legacy extensions that add panels via
Resizer instead of PanelManager).
- Add max-size support to Resizer in anticipation of PanelManager helping to
fix bug #2015.
…ment

* origin/master:
  Safe file delete.
  Style textarea like input.
  Install jasmine-node as needed by the jasmine-node task.
  Add check for no "functions" returned (as requested by code review).
  bumped pathmatch color down a notch based on peterflynn's feedback
  Change how QuickEdit helper function is passed from CodeHints to QuickEdit:
  Made .quicksearch-pathmatch darker so that it's visible
  Remove fallback for missing helper function (just fail if it's missing). It should never be missing.
  Fixes for comments on push'ed code:
  Tweaks to make some test cases pass.
  Use tern jump-to-definition search for QuickEdit.
…panels

resize or overall available `.content` height changes (window resize).
Change resizer min/max limits to operate in terms of panel outer height.
Copy link
Member Author

Choose a reason for hiding this comment

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

One thing I was on the fence about: it might be cleaner to move this line into PanelManager.triggerEditorResize(). That way PanelManager owns the editor-holder height entirely, so that when "editorAreaResize" is fired the editor area has indeed already been resized. And then EditorManager is just listening for that to refresh editors, nothing more...

Copy link
Member

Choose a reason for hiding this comment

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

Moving this into PanelManager.triggerEditorResize() sounds like a good idea. Did you try it? If everything seems to work, I'm inclined to take that change. If things don't work, there should be a FUTURE comment added to move it (and make it work :-) )

@ghost ghost assigned gruehle May 21, 2013
@gruehle
Copy link
Member

gruehle commented May 21, 2013

Reviewing

@peterflynn
Copy link
Member Author

Notes on backwards compatibility:

  • If you create a bottom panel the old way, injecting into DOM yourself, then calling Resizer.makeResizable() -- you'll get a deprecation warning but PanelManager will still "discover" the panel.
  • If you show/hide panels using the Resizer APIs -- everything continues to work. It's fully supported to use either the Resizer APIs or the methods on the new Panel object that PanelManager returns.
  • If you show/hide bottom panels using raw $.show()/.hide() + a call to EditorManager.resizeEditor() -- all current functionality (including the new "editorAreaResize" event and the Bottom panels get messed up if they are resized to the maximum height #2015 partial fix) will still work. However, you probably won't get the benefit of future panel-related fixes, like Scroll to cursor if hidden by JSLint panel #3371 or the rest of Bottom panels get messed up if they are resized to the maximum height #2015.
  • If you show/hide non-Resizer content in the editor area (e.g. the top panels added by RWDY and SVG Preview) by injecting into the DOM yourself & then calling EditorManager.resizeEditor() -- it will continue to work (and the new "editorAreaResize" event is still fired).

I tested a number of extensions to verify this:

  • brackets-sidebar-plus -- uses raw jQuery without calling resizeEditor(), but since this is a side panel it causes a bug only in the edge case of HTML menubar wrap (and that was true before this pull request too). (Filed When HTML menubar enabled, rendering issue at bottom of editor in some edge cases sathyamoorthi/brackets-sidebar-plus#4 to track that).
  • brackets-tab extension -- uses Resizer.toggle(), works fine.
  • brackets-jshint -- uses makeResizable() to create bottom panel, uses raw jQuery + resizeEditor() to show/hide. Gets a deprecation warning but works fine.
  • PGB -- similar, also works fine
  • brackets-display-shortcuts -- similar, also works fine

Copy link
Member

Choose a reason for hiding this comment

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

The JSDoc for createBottomPanel() says the id param should be in package format. Should this be "search.results"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will fix. I'll add a brackets. prefix too, to avoid collisions in case extension authors ignore the advice and use generic, short names.

@gruehle
Copy link
Member

gruehle commented May 21, 2013

Initial review complete.

createMockEditor() calls EditorManager.setEditorHolder() to inject mock DOM.
* Cleanup: move responsibility for setting editor-holder height from
EditorManager into PanelManager (cleaner division of labor).
document, so JS code hints unit tests can share SpecRunnerUtils' mock-Editor
creation code instead of duplicating it.
@gruehle
Copy link
Member

gruehle commented May 22, 2013

Looks good! Merging.

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.

Bottom panels get messed up if they are resized to the maximum height

3 participants