[webkit-reviews] review denied: [Bug 89586] Web Inspector: Add support for keyboard increment / decrement on numbers in attributes in Elements Panel : [Attachment 150882] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Vivek Galatage
<vivekgalatage at gmail.com>'s request for review:
Bug 89586: Web Inspector: Add support for keyboard increment / decrement on
numbers in attributes in Elements Panel
https://bugs.webkit.org/show_bug.cgi?id=89586

Attachment 150882: Patch
https://bugs.webkit.org/attachment.cgi?id=150882&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=150882&action=review


> Source/WebCore/inspector/front-end/UIUtils.js:279
> +WebInspector.alteredFloatNumber = function(number, event)

Please annotate with jsdoc and compile using compile-front-end.py.
This method is private (rename to _alteredFloatNumber)

> Source/WebCore/inspector/front-end/UIUtils.js:299
> +    if
(!String(result).match(WebInspector.StylesSidebarPane.CSSNumberRegex))

This looks like a bad dependency. Utility class should not depend on the styles
panel. You should move this check to the call site.

> Source/WebCore/inspector/front-end/UIUtils.js:305
> +WebInspector.handleUpOrDownValue = function(event, element,
suggestionHanlder, finishHandler)

Please annotate.

> Source/WebCore/inspector/front-end/UIUtils.js:334
> +	   number = WebInspector.StylesSidebarPane.alteredHexNumber(matches[2],
event);

Should also be moved into this class as a private member.