[webkit-reviews] review denied: [Bug 93581] Web Inspector: Feature Request - Adding mouse gesture for editing attribute values in elements/css panel : [Attachment 157428] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied sam <dsam2912 at gmail.com>'s
request for review:
Bug 93581: Web Inspector: Feature Request - Adding mouse gesture for editing
attribute values in elements/css panel
https://bugs.webkit.org/show_bug.cgi?id=93581

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

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


r- for small nits.

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:2530
>	       if (this._handleNameOrValueUpDown(event)) {

I would leave switch as is.

> Source/WebCore/inspector/front-end/TextPrompt.js:115
> +	   this._element.addEventListener("mousewheel", this._boundOnKeyDown,
false);

You should bind _handleNameOrValueUpDown explicitly (otherwise calling
boundOnKeyDown upon mouse event is misleading) and call event.consume(true)
from within _handleNameOrValueUpDown instead of doing preventDefault.

> Source/WebCore/inspector/front-end/UIUtils.js:315
> +WebInspector._modifiedHexValue = function(hexString, event, direction)

You should either derive direction and scale from the event or not pass event
into this method. Otherwise, you do half work in one place and another half in
another!

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

ditto