[webkit-reviews] review denied: [Bug 84235] Web Inspector: Does not have search navigation button for going through matches in either direction (prev, next) : [Attachment 137845] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied sam <dsam2912 at gmail.com>'s
request for review:
Bug 84235: Web Inspector: Does not have search navigation button for going
through matches in either direction (prev, next)
https://bugs.webkit.org/show_bug.cgi?id=84235

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

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


> Source/WebCore/inspector/front-end/WebKit.qrc:270
> +    <file>Images/searchNext.png</file>

You should add these images into WebCore.gypi as well.

> Source/WebCore/inspector/front-end/inspector.css:282
> +    background: -webkit-gradient(linear, left top, right top, from(rgb(230,
230, 230)), to(rgb(230, 230, 230)), color-stop(40%, rgb(252, 252, 252)));

You should not hard-code gradients - they don't look good on non-Mac /
different Mac OS versions.

> Source/WebCore/inspector/front-end/inspector.css:291
> +    background: -webkit-gradient(linear, left top, right top, from(rgb(128,
128, 128)), to(rgb(164, 164, 164)), color-stop(25%, rgb(164, 164, 164)));

ditto

> Source/WebCore/inspector/front-end/inspector.css:297
> +    margin: 5px 0px 0px 5px;

These margins look a bit large to me.

> Source/WebCore/inspector/front-end/inspector.html:215
> +	       <div class="toolbar-search-nav-ctrl">

We prefer to not add markup here, but to create it lazily using DOM api.