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

core-ai-bot commented 3 years ago

Issue by gruehle Thursday Nov 01, 2012 at 16:25 GMT Originally opened as https://github.com/adobe/brackets/issues/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.

core-ai-bot commented 3 years ago

Comment by jbalsas Friday Nov 02, 2012 at 11:56 GMT

Hi,

A couple things I've noticed about this:

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.

core-ai-bot commented 3 years ago

Comment by pthiess Tuesday Nov 06, 2012 at 00:21 GMT

Reviewed

core-ai-bot commented 3 years ago

Comment by peterflynn Monday Nov 19, 2012 at 19:55 GMT

There are several intertwined bugs here:

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 fits
            var maxSize = $("#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.

core-ai-bot commented 3 years ago

Comment by peterflynn Monday Nov 19, 2012 at 19:56 GMT

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.

core-ai-bot commented 3 years ago

Comment by gruehle Monday Nov 19, 2012 at 21:42 GMT

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.

core-ai-bot commented 3 years ago

Comment by njx Monday Nov 26, 2012 at 19:35 GMT

Leaving open for the simple fix for sprint 17. We can do more later.

core-ai-bot commented 3 years ago

Comment by redmunds Tuesday Nov 27, 2012 at 16:06 GMT

FBNC@gruehle Please verify partial fix submitted in pull request #2223. When confirmed, remove FBNC and Sprint 17 tags, and leave Open.

core-ai-bot commented 3 years ago

Comment by gruehle Tuesday Nov 27, 2012 at 17:39 GMT

Yes, the worst case is now fixed. Removing FBNC and Sprint 17. Keeping Medium Priority since the remaining issues are still pretty bad.

core-ai-bot commented 3 years ago

Comment by njx Wednesday Jan 09, 2013 at 17:48 GMT

Nominating for sprint 19.

core-ai-bot commented 3 years ago

Comment by njx Monday Jan 14, 2013 at 19:43 GMT

Moving out to sprint 20--doesn't seem as high pri as other sprint 19 bugs and we're getting low on time.

core-ai-bot commented 3 years ago

Comment by peterflynn Friday Feb 08, 2013 at 18:21 GMT

Moving to sprint 21 after discussion -- not as important as the ongoing CMv3 issues.

core-ai-bot commented 3 years ago

Comment by peterflynn Wednesday Feb 27, 2013 at 18:15 GMT

Moving to sprint 22 since the new topcoat toolbar story will probably affect how panel layout is managed.

core-ai-bot commented 3 years ago

Comment by pthiess Wednesday Apr 10, 2013 at 18:19 GMT

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.

core-ai-bot commented 3 years ago

Comment by njx Wednesday Apr 24, 2013 at 17:20 GMT

Further punted.@peterflynn to write up proposal for fix

core-ai-bot commented 3 years ago

Comment by peterflynn Tuesday May 21, 2013 at 19:58 GMT

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...).

core-ai-bot commented 3 years ago

Comment by njx Monday Jun 03, 2013 at 18:12 GMT

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.

core-ai-bot commented 3 years ago

Comment by njx Monday Jul 08, 2013 at 19:02 GMT

Created a backlog card for bottom panel UI and linked this bug from it: https://trello.com/c/eVoQHUJj. Closing as move to backlog.

brackets-archive / bracketsIssues

[CLOSED] Bottom panels get messed up if they are resized to the maximum height #1950