[Webkit-unassigned] [Bug 84235] Web Inspector: Does not have search navigation button for going through matches in either direction (prev, next)

[bugzilla-daemon at webkit.org]

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





--- Comment #15 from sam <dsam2912 at gmail.com>  2012-04-19 22:32:49 PST ---
Thanks Pavel for comments.

(In reply to comment #13)
> (From update of attachment 137908 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=137908&action=review
> 
> > Source/WebCore/WebCore.gypi:6555
> > +            'inspector/front-end/Images/searchBtnBck.png',
> 
> We try to not abbreviate in WebKit. searchButtonBack, etc.
> 
corrected.
> > Source/WebCore/inspector/front-end/SearchController.js:40
> > +    this._searchControlBoxElement = document.getElementById("toolbar-search-nav-ctrl");
> 
> -navigation-control
> 

corrected.
> > Source/WebCore/inspector/front-end/SearchController.js:258
> > +    _onNextBtnSearch: function(event)
> 
> No abbreviations please.
> 
corrected.
> > Source/WebCore/inspector/front-end/SearchController.js:264
> > +    _onPrevBtnSearch: function(event)
> 
> ditto
>
corrected. 
> > Source/WebCore/inspector/front-end/SearchController.js:336
> > +    _createSearchNavBtn: function(direction) {
> 
> ditto. Also, you should place { on the next line.
>
my bad!, corrected. 
> > Source/WebCore/inspector/front-end/SearchController.js:340
> > +        switch(direction) {
> 
> Do not offset case from switch, use 4 characters for indentation within case blocks.
> 
corrected.
> > Source/WebCore/inspector/front-end/SearchController.js:342
> > +                       searchNavControlElement.setAttribute("title" , "Search Previous");
> 
> You should use WebInspector.UIString("Search Previous") and add the string itself into localizedStrings.js (WebCore/English.lproj).
>
Thank you, corrected. 
> > Source/WebCore/inspector/front-end/SearchController.js:359
> > +    _populateSearchNavButtons: function() {
> 
> { on the next line
> 
> > Source/WebCore/inspector/front-end/inspector.css:279
> > +    background-image: url('Images/searchBtnBck.png');
> 
> Sorry for not being clear in my previous review. I don't think images will make it better - they are still not in line with the theme the OS might use for the inspector. We try to only use thin borders and/or icons with (semi-)transparent background when we make new controls.

I have put in border with no background. Is it ok?

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