[Webkit-unassigned] [Bug 84708] Web Inspector: Shortcuts screen UI polish

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=84708


Pavel Feldman <pfeldman at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #138989|1                           |0
        is obsolete|                            |




--- Comment #6 from Pavel Feldman <pfeldman at chromium.org>  2012-04-26 06:55:22 PST ---
(From update of attachment 138989)
View in context: https://bugs.webkit.org/attachment.cgi?id=138989&action=review

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

You need to explain what has happened and why here (enumerate changes you've done briefly).

> Source/WebCore/inspector/front-end/KeyboardShortcut.js:64
> +    Left: { code: 37, name: "<Left>" },           // also NUM_WEST

Make sure these are getting localized (get into English.lproj/localizedStrings.js)

> Source/WebCore/inspector/front-end/ShortcutsScreen.js:82
> +        var minMax = totalHeight;

It is not clear what minMax stands for.

> Source/WebCore/inspector/front-end/ShortcutsScreen.js:83
> +        var minMaxIdx = -1;

Please do not use abbreviations (should be Index)

> Source/WebCore/inspector/front-end/ShortcutsScreen.js:86
> +        for (idx = 0; idx < orderedSections.length; ++idx) {

for (var i = 0; i < ... ++i) {
}

> Source/WebCore/inspector/front-end/ShortcutsScreen.js:-80
> -        // This manual layout ugliness should be gone once WebKit implements

The underlying issue it not fixed though - what if user resizes the screen?

> Source/WebCore/inspector/front-end/ShortcutsScreen.js:106
> +            for (idx = stopIndices[columnIndex - 1]; idx < stopIndices[columnIndex]; ++idx) {

No need for { } around single line blocks.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.