You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Sep 6, 2021. It is now read-only.
Repository navigation
This repository was archived by the owner on Sep 6, 2021. It is now read-only.
Bottom panels get messed up if they are resized to the maximum height #2015
[Edit: The behavior as described in these steps was fixed in #2223. However, there are related issues that still remain. Please read all bug comments before closing.]
Steps to repro:
Open the Getting Started project (or any project, for that matter)
Do a "Find in Files" for "body"
Resize the results panel as tall as it can go. There should still be a few pixels of editor viewable, but no text will be shown.
Close the panel
Do another "Find in Files" operation
Results:
The search results panel ends up overlapping the status bar and cannot be resized. The only way to get the panel back is to trash prefs.
You can actually get the panel back, but you need to do it backwards. Start making it smaller until the height of the panel is less than the editor's height and the panel will pop back up again. (This should make debugging this less painful...)
Note how as the panel keeps growing the status bar starts sliding down once it has reached the maximum height.
The same error can be reproduced by (steps 2 and 3 can be switched in order):
Resize the panel to any given height.
Close the panel
Resize Brackets window so that the height is smaller than the panel.
Reopen the panel
It looks like some css or layout changes might help here. For example, the data-forcemargin for the sidebar resizer could be updated to push the status bar the same way we do with the content if required.
Also limiting a maximum height while resizing the panels (on resizing and on showing) helps, but this would probably require a some API changes in Resizer.
Resizer allows the panel to be resized past the largest size that will fit (i.e. the size at which the editor is 0 height). The repro steps above almost guarantee this, since it's hard to stop your drag perfectly the instant editor height reaches 0. You'll typically overshoot a little.
EditorManager._calcEditorHeight() returns a negative number when the panel is too large. jQuery.height() no-ops on negative numbers (not sure why it doesn't clip to zero instead but hey, since when do jQuery APIs make any sense?). So this causes the Editor to not get resized at all when an overly-large panel is reshown later -- resulting in the nasty behavior below where the panel is barely visible and can't be fixed via resizing (since it's already too tall), and the status bar is pushed off the bottom.
If you make the panel huge, close it, resize the Brackets window smaller, then reopen the panel, it will now have a size that can't fit in the reduced available space. The panel will stick pretty far off the bottom, so the bottom of its scrollbar (if any) will be inaccessible and the status bar will be completely hidden. Also, to resize the panel you have to drag pretty far before it has any visible effect (because of how much was sticking off the bottom).
If you make the panel huge, leave it open, and resize the Brackets window smaller you'll see all the same bugs as (3).
So as far as fixes:
The nastiest parts of (2) are easy to fix by adding a Math.max() on the return value in _calcEditorHeight() -- the panel will then be positioned at the top as expected and resizing it will work properly (though the panel will be a tiny bit too tall from the overshoot, clipping the bottom few px of the status bar off the bottom -- a much less severe glitch).
(3) and the remainder of (2) can be fixed by calculating the max available space in the Resizer "show" helper and capping the panel size accordingly. We then also have to change FindInFiles to use Resizer.show()/hide() instead of the generic jQuery function (and ditto for JSLintUtils).
(4) would require doing the same check during window resize, which may be trickier to do in a performant way... and perhaps should be managed more centrally through EditorManager.
Here's the code I added to the "show" helper for (3):
// Window size may have changed since panel last visible. Make sure it still fitsvarmaxSize=$("#editor-holder").height();if(elementSize>maxSize){elementSize=maxSize;elementSizeFunction.apply($element,[maxSize]);}
This isn't ideal since it hardcodes an assumption that the panel sits in the .content vertical stack. We could add fancier logic to detect whether that's actually true or not. Might want to encapsulate some of it in EditorManager, though.
So for Sprint 17 since we're sort of in a crunch, I would suggest only doing my first bullet -- the simple fix in EditorManager that fixes the worst aspects of the bug.
The simple EditorManager fix sounds good for now. As you say, this will resolve the situation where you can't get the panel back unless you trash prefs, which is the most important aspect to fix.
PR #3943 fully fixes (1) above. Here's a rough sketch of the remaining work:
To fix (3) and the remainder of (2), Resizer needs to respect the panel maxsize when showing a hidden panel. (In the $element.data("show", ...) handler we can just clip elementSize appropriately -- easy fix).
There's also a variant of (3) when panels are initialized at startup, restoring sizes from prefs. I think we'd have to do something inside the if (elementPrefs) block in Resizer.
To fix (4), PanelManager needs to reduce panel sizes during/after a window resize. Not sure what logic it should use for deciding which panels to shrink (panels can have min sizes, so we'd have to factor that in too -- and then there's the question of what to do when the window is simply too small to fit all the fixed/min-size content...).
Reviewed. Since the worst issues here are fixed, and panels were never an official feature anyway, we decided that we should call this "move to backlog", and create a card for creating an official panel API that includes things like whether we want tabs in panels, what their sizing behavior should be, etc.
Activity
jbalsas commentedon Nov 2, 2012
Hi,
A couple things I've noticed about this:
It looks like some css or layout changes might help here. For example, the
data-forcemarginfor the sidebar resizer could be updated to push the status bar the same way we do with the content if required.Also limiting a maximum height while resizing the panels (on resizing and on showing) helps, but this would probably require a some API changes in
Resizer.pthiess commentedon Nov 6, 2012
Reviewed
peterflynn commentedon Nov 19, 2012
There are several intertwined bugs here:
So as far as fixes:
Here's the code I added to the "show" helper for (3):
This isn't ideal since it hardcodes an assumption that the panel sits in the
.contentvertical stack. We could add fancier logic to detect whether that's actually true or not. Might want to encapsulate some of it in EditorManager, though.peterflynn commentedon Nov 19, 2012
So for Sprint 17 since we're sort of in a crunch, I would suggest only doing my first bullet -- the simple fix in EditorManager that fixes the worst aspects of the bug.
gruehle commentedon Nov 19, 2012
The simple EditorManager fix sounds good for now. As you say, this will resolve the situation where you can't get the panel back unless you trash prefs, which is the most important aspect to fix.
njx commentedon Nov 26, 2012
Leaving open for the simple fix for sprint 17. We can do more later.
Partial fix for #2015 (Bottom panels get messed up if resized to max …
Merge pull request #2223 from adobe/pflynn/huge-panels-fix
redmunds commentedon Nov 27, 2012
FBNC @gruehle Please verify partial fix submitted in pull request #2223. When confirmed, remove FBNC and Sprint 17 tags, and leave Open.
gruehle commentedon Nov 27, 2012
Yes, the worst case is now fixed. Removing FBNC and Sprint 17. Keeping Medium Priority since the remaining issues are still pretty bad.
njx commentedon Jan 9, 2013
Nominating for sprint 19.
njx commentedon Jan 14, 2013
Moving out to sprint 20--doesn't seem as high pri as other sprint 19 bugs and we're getting low on time.
peterflynn commentedon Feb 8, 2013
Moving to sprint 21 after discussion -- not as important as the ongoing CMv3 issues.
peterflynn commentedon Feb 27, 2013
Moving to sprint 22 since the new topcoat toolbar story will probably affect how panel layout is managed.
pthiess commentedon Apr 10, 2013
Moving to Sprint 24 as per team's decision in today's standup. @peterflynn is going to write up a proposal for this case, we'll follow up on.
njx commentedon Apr 24, 2013
Further punted. @peterflynn to write up proposal for fix
Initial PanelManager checkin:
Fix part of #2015 by keeping panel resize limits up to date as other …
peterflynn commentedon May 21, 2013
PR #3943 fully fixes (1) above. Here's a rough sketch of the remaining work:
$element.data("show", ...)handler we can just clipelementSizeappropriately -- easy fix).if (elementPrefs)block in Resizer.Initial PanelManager checkin:
Fix part of adobe#2015 by keeping panel resize limits up to date as o…
njx commentedon Jun 3, 2013
Reviewed. Since the worst issues here are fixed, and panels were never an official feature anyway, we decided that we should call this "move to backlog", and create a card for creating an official panel API that includes things like whether we want tabs in panels, what their sizing behavior should be, etc.
njx commentedon Jul 8, 2013
Created a backlog card for bottom panel UI and linked this bug from it: https://trello.com/c/eVoQHUJj. Closing as move to backlog.