[CLOSED] Fix for issue #1122

[core-ai-bot]

Issue by jbalsas
Sunday Sep 16, 2012 at 10:46 GMT
Originally opened as adobe/brackets#1661

---

This pull request provides a possible fix for issue #1122.

It adds resize functionality for the two existing bottom panels ("jslint errors" and "find in files").

Knwon issues

• If a panel is resized beyond the main toolbar, it keeps growing. If the mouse is released within the toolbar the panel height is greater than the main view height. When starting the panel resize again, there is an offset between the handler and the panel height due to this.
• If both panels are opened, resizing the search panel until both the panels cover the entire editor and keep going causes the same effect as the previous issue.
• If both panels are opened, resizing the jslint panel beyond the main toolbar causes the find panel to shift downwards as the first one keeps growing.

This issues could be solved but it will require to introduce some dependencies between panels or dom elements and betwen panels and the toolbar element.

---

jbalsas included the following code: https://github.com/adobe/brackets/pull/1661/commits

[core-ai-bot]

Comment by redmunds
Monday Sep 17, 2012 at 17:57 GMT

---

Thanks for submitting this code. We have an internal milestone that we're trying to hit this week, so I might not get to reviewing this code until next week.

[core-ai-bot]

Comment by redmunds
Wednesday Sep 26, 2012 at 15:21 GMT

---

This is cool, but I'm seeing a couple problems with this on Win7. I posted a pic here: https://twitter.com/randyedmunds/status/250976078926077952/photo/1 (Let me know if the resolution got messed up and I can e-mail the pic).

Notice in the upper (JSLint) panel that after increasing the height of the panel, I still see the same number of lines of JSLint errors displayed (with a vertical scrollbar). The number of lines viewed should be increased to fill the height of the panel.

Notice the lower (Find in Files) panel that the number of lines are increased with the height, but on the lower end of the scrollbar, the down-arrow button is mostly off the screen.

[core-ai-bot]

Comment by jbalsas
Wednesday Sep 26, 2012 at 22:34 GMT

---

Hi!

I think I messed up the jslint results panel resize when doing some cleaning before the pull request... fixed now :)

About the other issue, scrollbars on Mac are somehow subtler so this was harder to spot than on windows. I was using height() to get the toolbars height, which wasn't considering paddings and margins. I've changed it to outerHeight() instead. Please, check it to see if is ok now.

[core-ai-bot]

Comment by redmunds
Thursday Sep 27, 2012 at 00:37 GMT

---

Excellent. Both of the problems mentioned above are fixed. I am noticing another problem with releasing mouse capture. Do this:

1. Open JSLint panel (it only happens with JSLint panel, either by itself or with FiFs panel)
2. Drag height up as high as you can (until editor disappears)
3. Release left mouse button
4. Now move mouse down over JSLint panel

Results: JSLint panel height starts decreasing as if mouse button was never released, so it seems to still have the capture.

Expected: Obviously, releasing the mouse should stop the drag, but I'm not sure if I like how the entire editor can be covered up. I am wondering if enforcing a minimum height on the editor will fix both problems?

[core-ai-bot]

Comment by jbalsas
Thursday Sep 27, 2012 at 17:59 GMT

---

I basically thought of this as a temporary fix while getting a crack at the tabbed panel solution discussed in https://groups.google.com/forum/?fromgroups=#!topic/brackets-dev/PqEbuhw7k6c. I feel, that would be the best way to go.

We should of course improve this if it is to be made part of Brackets in the meantime.

About the behavior you saw, I think it only happens when you release the mouse outside the Brackets window (also over the app toolbar). It also happens for the FiFs panel. I'll try to fix this later today.

About enforcing a minimum height for the editor, I did try it, but it didn't convince me at all. There are lots of issues to be addressed for that:

1. Dependencies between panels

We'd need to check the height of all visible panels to get the available space. I didn't want to create any dependencies between panels. Having a utils/Resizer class that tracks all panels could help with this...

2. Resizing the window

• With one panel

If we start resizing the whole window... what should happen when the minimum height for the editor is reached? Should the panel shrink to leave place for the editor?

• With two or more panels

If we start resizing the whole window, when reaching the minimum height for the editor, should the panels shrink? Should all panels shrink in the same proportion? Ones more than others?

3. Resizing the panels

If we start resizing a panel and the maximum height for all panels is reached, should we stop all resizing or should other panels give height away for or it?

4. Opening panels

If we try to open a panel and there is not enough space for the saved height of the panel, should we set it to the maximum available height? What if no space at all is left?

I think solving all these issues may be a lot of work that could be better spent on the tabbed panel solution if that is the final goal. I'm willing to work in both directions in any case, so just point at what should we do, please :)

[core-ai-bot]

Comment by redmunds
Thursday Sep 27, 2012 at 22:09 GMT

---

This resizing code will still be needed if the tabbed interface is implemented.

Enforcing a minimum height on the editor is just a suggestion for a possible solution, but it's not required. I does not seem to be worth the effort.

But, the mouse not releasing the capture needs to be solved. Can the "dragleave" event be used to release the mouse at the point when mouse exits window?

[core-ai-bot]

Comment by jbalsas
Thursday Sep 27, 2012 at 22:36 GMT

---

I think dragleave is for drag operations, which we aren't really doing. However, jquery .mouseleave() should do the trick.(Note that this is currently also happening for the sidebar...)

I'll check and fix this and refactor the resize as suggested.

[core-ai-bot]

Comment by jbalsas
Monday Oct 01, 2012 at 07:50 GMT

---

I've created a a new utils/Resizer as discussed.

It scans the DOM on AppInit.htmlReady for ver-resizable and hor-resizable classes and adds the resize handlers for those elements. It expects a top-resizer or bottom-resizer class in the case of vertical resizing to identify where to put the handler (both could work at the same time in the future). The same way, it expects a left-resizer or right-resizer for horizontal resizing. Finally, it also looks for an optional element with a content-resizable class that should be resized at the same time the parent does, to fill the available space.

The process creates a promise that the resizable modules can use to get notifications of the resizing process using the progress method. The associated deferred objetct never gets resolved. It gets notified any time the resize starts, updates or ends. This way, modules can do performance improvements (like the sidebar does, hiding elements when resizing starts), custom resize on progress, or saving preferences and restoring elements when resize ends.

Known Issues

• The code works right now for vertical resizing and resizer on top. Bottom vertical resizing as well as horizontal resizing would need small modifications. I think this could be done in future requests.
• The scan for classes is triggered on AppInit.htmlReady, and modules can retrieve their resize promises on AppInit.appReady. This prevents extensions to benefit from this and force them to explicitly call makeResizable. We could solve this by triggering the scan on AppInit.appReady instead and create a new event Resizer.ready or AppInit.resizerReady to be triggered afterwards.

[core-ai-bot]

Comment by redmunds
Monday Oct 01, 2012 at 20:37 GMT

---

Excellent! This is much better. Now that the high-level design is in place, I'm going to do one last review of the code details.

[core-ai-bot]

Comment by redmunds
Monday Oct 01, 2012 at 22:13 GMT

---

Done with review.

[core-ai-bot]

Comment by redmunds
Monday Oct 08, 2012 at 17:36 GMT

---

@jbalsas I see you pushed some changes since my last comment, so I am just checking to see where we're at with this one. I want to get this change into master so I can use it! Also, there are conflicts with master, so you'll need to merge the latest changes to master to this branch. Thanks.

[core-ai-bot]

Comment by jbalsas
Monday Oct 08, 2012 at 19:14 GMT

---

@redmunds Yes, I added your final suggestions the other day in London (@peterflynn helped me with those ;)) Please, tell me if there's anything left to add.

I'll merge the changes to master later tonight.

[core-ai-bot]

Comment by redmunds
Monday Oct 08, 2012 at 20:15 GMT

---

@jbalsas Glad I asked :) Please post a comment with@redmunds when passing something back so I don't wait unnecessarily.

This looks good. 2 very minor other changes:

• line 190 of Resizer.js has a typo -- it should be "right" not "bottom".
• Totally optional, but do you mind renaming "ver-resizable" to be "vert-resizable", and "hor-resizable" to be "horz-resizable"? Seems much quicker to remember what they're for.

[core-ai-bot]

Comment by jbalsas
Monday Oct 08, 2012 at 20:36 GMT

---

@redmunds Done and done ;)

[core-ai-bot]

Comment by redmunds
Monday Oct 08, 2012 at 20:43 GMT

---

Looks good. Thanks for sticking with this one! Merging.