[Webkit-unassigned] [Bug 85061] Web Inspector: Add "Show keymap" button to settings screen.

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Andrey Kosyakov <caseq at chromium.org>  2012-04-28 03:40:57 PST ---
(From update of attachment 139199)
View in context: https://bugs.webkit.org/attachment.cgi?id=139199&action=review

I'm not fan of the UI approach and "Miscellaneous" section in particular. How about moving it to the title bar, perhaps as a link or some iconic button?

> Source/WebCore/inspector/front-end/SettingsScreen.js:109
> +    function showKeyMap()
> +    {
> +        WebInspector.shortcutsScreen.show();
> +    }

I'd rarther pass WebInspector.shortcutsScreen.bind(WebInspector.shortcutsScreen) to createButton.

> Source/WebCore/inspector/front-end/SettingsScreen.js:165
> +        button.appendChild(document.createTextNode(WebInspector.UIString(name)));

Drop WebInspector.UIString(), you're already calling it on the call site (which is the preferred approach, btw)

>> Source/WebCore/inspector/front-end/SettingsScreen.js:241
>> +        var div = document.createElement("div");
> 
> Why do you need a wrapper around "select"? Can the "help-shadowbox" class be applied directly to the "select" element?

Why did this change? Is this "minor LnF" change? Please split it into a separate patch.

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