Bug 14265 - Cannot resize columns in webinspector
Summary: Cannot resize columns in webinspector
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 523.x (Safari 3)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-06-20 22:51 PDT by Niels Leenheer (HTML5test)
Modified: 2007-10-25 19:01 PDT (History)
4 users (show)

See Also:


Attachments
Patch (5.17 KB, patch)
2007-06-22 22:12 PDT, Matt Lilek
timothy: review-
Details | Formatted Diff | Diff
Patch v2 (5.63 KB, patch)
2007-06-24 12:16 PDT, Matt Lilek
no flags Details | Formatted Diff | Diff
Resizing of styles sidebar (6.31 KB, patch)
2007-08-10 04:44 PDT, Sjoerd Tieleman (tieleman)
no flags Details | Formatted Diff | Diff
Resizing of styles sidebar v2 (7.19 KB, patch)
2007-08-10 06:06 PDT, Sjoerd Tieleman (tieleman)
aroben: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Niels Leenheer (HTML5test) 2007-06-20 22:51:16 PDT
It is not possible to resize the left column even though there is a resize widget on the right-bottom of the left column. It simply doesn't do anything.
Comment 1 Matt Lilek 2007-06-22 22:12:34 PDT
Created attachment 15190 [details]
Patch
Comment 2 Timothy Hatcher 2007-06-22 22:31:17 PDT
Comment on attachment 15190 [details]
Patch

+function dividerDragEnd(element, dividerDrag, dividerDragEnd, event) 

You should put these functions in the WebInspector "namespace" like the other functions.

Also the CSS for the sidebarResizeWidget should have cursor: col-resize;

Otherwise this is a r=me.

We still want to make resizing work by hovering over the divider line.
Comment 3 Matt Lilek 2007-06-24 12:16:32 PDT
Created attachment 15207 [details]
Patch v2
Comment 4 Timothy Hatcher 2007-06-24 12:21:42 PDT
Comment on attachment 15207 [details]
Patch v2

r=me
Comment 5 Matt Lilek 2007-06-24 12:24:55 PDT
r23753
Comment 6 Matt Lilek 2007-06-24 12:26:45 PDT
Reopening until other areas are resizable
Comment 7 Mark Rowe (bdash) 2007-06-25 03:25:03 PDT
Comment on attachment 15207 [details]
Patch v2

Clearing review flag on landed patch to get it out of the commit queue.
Comment 8 Sjoerd Tieleman (tieleman) 2007-08-10 04:44:24 PDT
Created attachment 15897 [details]
Resizing of styles sidebar

This enables resizing the right-hand column and makes sure that when dragging is done the relevant areas will remain the correct sizes when resizing the window.
Comment 9 Sjoerd Tieleman (tieleman) 2007-08-10 06:06:24 PDT
Created attachment 15899 [details]
Resizing of styles sidebar v2

Enable the resizing of the styles sidebar in the inspector. Fixes a bug introduced in previous patch where the resize widget would be unaccessible when the sidebar contains a scrollbar.
Comment 10 Matt Lilek 2007-08-10 06:18:19 PDT
(In reply to comment #9)
> Created an attachment (id=15899) [edit]
> Resizing of styles sidebar v2
> 
> Enable the resizing of the styles sidebar in the inspector. Fixes a bug
> introduced in previous patch where the resize widget would be unaccessible when
> the sidebar contains a scrollbar.
> 

I'm not a reviewer so I can't r- this, but I do see a few problems:
- The width of the resized column should be constrained, right now you can drag it all the way past the edge of the window in both directions.

- While not the most intuitive way to resize things, the borders of the columns should be able to resize the column as well (like in iTunes). You need this for the right hand column especially, since right now the bread crumb trail doesn't restrict itself to the available space and inspecting a deeply nested area of the page won't show the resize handle.

- You can't rely on IDs to resize the right column because there could be multiple documents which all have their own sidebar. When there are multiple documents, we don't reuse the same right hand sidebar and source view thingy, but create new ones because of the back/forward feature. Try going to a page with frames (ie: GMail) and you'll see that you can only resize one column.

- In cases like GMail that have multiple documents, resizing one column should resize them all so that the size doesn't jump when switching between documents. 

- You should fill in your email address in the ChangeLog.

* Unrelated and this is something for a followup patch, but we should really rename the left-hand sidebar to sourcelist or something else, because not only does having multiple meanings of "sidebar" in the source make it confusing to work on at times, but confusing to talk about as well.
Comment 11 Adam Roben (:aroben) 2007-08-10 11:04:59 PDT
Comment on attachment 15899 [details]
Resizing of styles sidebar v2

There are some extraneous whitespace changes towards the beginning of the patch that shouldn't be in here.

r- until you address Matt's comments.
Comment 12 Timothy Hatcher 2007-08-10 11:41:07 PDT
Comment on attachment 15899 [details]
Resizing of styles sidebar v2

+            this.views.dom.contentElement.sideContentElement.id = "treeOutline";
+            this.views.dom.contentElement.mainResizeWidget.id = "mainResizeWidget";
+            this.views.dom.contentElement.sidebarElement.id = "propertiesSidebar";

You cannot assign id's to these elements. These elements are created for each file in the inspector. And id's need to be only used once in the document. You will not get the element you expect when doing document.getElementById().

+            document.getElementById("mainResizeWidget").addEventListener("mousedown", WebInspector.mainResizerDragStart, true);

This code should be:

             this.views.dom.contentElement.mainResizeWidget.addEventListener("mousedown", WebInspector.mainResizerDragStart, true);

I also don't think we should have a resize widget in the corner. Resizing by the divider line is all we need. The divider line is only 1px wide, so a drag area of 5px (2px on each side, 1px in the center) would be good. Resizing by the divider is something we are still missing on the file list sidebar, mainly because this isn't trivial to fix.
Comment 13 Sjoerd Tieleman (tieleman) 2007-08-11 07:12:05 PDT
Ok. Work to be done than. ;-)
For dragging by the divider, would something like this be good?

http://www.sjoerdtieleman.nl/divider/

This basically puts an absolute positioned div with a 5px wide png background on the resizable area. The png has 2px on the left and 2px on the right of transparancy, so the divider appears 5px wide for the mouse, but only 1px visually.
If such a thing were to be done, would the current resize widgets be removed alltogether?
Comment 14 Timothy Hatcher 2007-08-11 08:29:14 PDT
Yeah that is the basic idea. Except there is no need for a 5px image. The divv can just be 5px wide and have no style, so it is transparent. I will just be used for the mousedown event, etc. (Transparent divs still get events,)

I think the resize widget can be left out for the right sidebar.
Comment 15 Adam Roben (:aroben) 2007-08-11 12:59:27 PDT
You should have a look at how resizing already works for the left column. I should think that a lot of the code could be shared.
Comment 16 Adam Roben (:aroben) 2007-10-25 19:01:09 PDT
Fixed in r26922 <http://trac.webkit.org/projects/webkit/changeset/26922>