Bug 55238

Summary: Web Inspector: Resizing columns in the network panel is weird
Product: WebKit Reporter: Matt Lilek <dev+webkit>
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, commit-queue, joepeck, keishi, loislo, pfeldman, pmuellr, rik, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Pathc
pfeldman: review-
Patch with style fixes none

Description Matt Lilek 2011-02-25 11:10:44 PST
Resizing the columns in the Network panel is weird.  Resizing a column only changes the amount of space between the columns on either side of the resizer instead of adjusting the whole tableview (the behavior of native TableViews on OS X).

For example, this makes is really hard to resize the Name column to see the full names of long resources.

I don't remember if this is how tableviews behave on Windows and don't have a machine handy to test, but we should match the host platform's behavior if that's the case.
Comment 1 tonistiigi 2011-05-18 12:50:20 PDT
Created attachment 93971 [details]
Patch

Includes resizing methods: nearest, last and first. Native NSTableView also has sequential an reverse sequential(that probably have the best effect) but adding those would mean adding minimum and maximum width ranges to columns instead of single fixed values.
Comment 2 WebKit Review Bot 2011-05-18 12:53:13 PDT
Attachment 93971 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/ChangeLog:4:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 3 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 tonistiigi 2011-05-18 13:00:51 PDT
Created attachment 93973 [details]
Patch
Comment 4 WebKit Review Bot 2011-05-18 13:02:41 PDT
Attachment 93973 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/ChangeLog:4:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 tonistiigi 2011-05-18 13:08:10 PDT
Created attachment 93976 [details]
Pathc
Comment 6 Pavel Feldman 2011-05-19 04:32:00 PDT
Comment on attachment 93976 [details]
Pathc

View in context: https://bugs.webkit.org/attachment.cgi?id=93976&action=review

There is a series of style nits, but otherwise looks good. Thanks for fixing this!

> Source/WebCore/inspector/front-end/DataGrid.js:927
> +        if (typeof this._resizeMethod==="undefined")

Nit: this should be:

typeof this._resizeMethod === "undefined"

> Source/WebCore/inspector/front-end/DataGrid.js:965
> +        if(this.resizeMethod==WebInspector.DataGrid.ResizeMethod.Last)

Spaces around == and before ( as well.

> Source/WebCore/inspector/front-end/DataGrid.js:966
> +        {

Style for if is
if (foo === bar) {
...
}

> Source/WebCore/inspector/front-end/DataGrid.js:969
> +        else if(this.resizeMethod==WebInspector.DataGrid.ResizeMethod.First)

ditto

> Source/WebCore/inspector/front-end/DataGrid.js:971
> +            leftEdgeOfPreviousColumn+=firstRowCells[leftCellIndex].offsetWidth - firstRowCells[0].offsetWidth;

Spaces around += are missing.

> Source/WebCore/inspector/front-end/DataGrid.js:1011
> +    Nearest:    'nearest',

No need to align values into a column.
Please use " instead of '
Comment 7 tonistiigi 2011-05-19 08:30:35 PDT
Created attachment 94071 [details]
Patch with style fixes
Comment 8 Pavel Feldman 2011-05-19 10:47:43 PDT
Comment on attachment 94071 [details]
Patch with style fixes

View in context: https://bugs.webkit.org/attachment.cgi?id=94071&action=review

It looks good now, thanks. One nit you don't have to fix since it is minor + would require re-review. I am marking this change as cq+ which means it'll land automatically.

> Source/WebCore/inspector/front-end/DataGrid.js:965
> +        if (this.resizeMethod == WebInspector.DataGrid.ResizeMethod.Last) {

Nit: single liners do not need {}
Comment 9 WebKit Commit Bot 2011-05-19 11:15:13 PDT
Comment on attachment 94071 [details]
Patch with style fixes

Clearing flags on attachment: 94071

Committed r86855: <http://trac.webkit.org/changeset/86855>
Comment 10 WebKit Commit Bot 2011-05-19 11:15:19 PDT
All reviewed patches have been landed.  Closing bug.