[Webkit-unassigned] [Bug 71262] Web Inspector: Add colorpicker functionality to color swatches in Styles Sidebar

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 8 07:34:14 PST 2011


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





--- Comment #29 from Alexander Pavlov (apavlov) <apavlov at chromium.org>  2011-12-08 07:34:13 PST ---
(From update of attachment 117092)
View in context: https://bugs.webkit.org/attachment.cgi?id=117092&action=review

Overall the code looks good, I'll defer it to a real reviewer to comment on more subtle issues.

> Source/WebCore/inspector/front-end/Spectrum.js:49
> +    alphaLabel.textContent = WebInspector.UIString("alpha") + ": ";

add to English.lproj/localizedStrings.js

> Source/WebCore/inspector/front-end/Spectrum.js:64
> +    colorLabel.textContent = WebInspector.UIString("color") + ": ";

add to English.lproj/localizedStrings.js

> Source/WebCore/inspector/front-end/Spectrum.js:359
> +        

extra blank line

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1644
> +                    swatchElement.title = WebInspector.UIString("Click to open a colorpicker");

add to English.lproj/localizedStrings.js

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1677
> +                    if (e.altKey) {

Remove braces for the one-line block.

> Source/WebCore/inspector/front-end/StylesSidebarPane.js:1695
> +                    var format;

optional: While we are here, you can remove this var altogether and simply return the required value from every alternative (that way you'll have to remove "else" in "else if", so that they will look like
if (.....)
    return cf.something;
if (.....)
    return cf.someOtherThing;
...

> Source/WebCore/inspector/front-end/inspector.css:2681
> +  position: relative; 

bad indentation for the 3 properties. Make sure you always use 4 spaces, not [a mixture of spaces and] tabs

> Source/WebCore/inspector/front-end/inspector.css:2750
> +   border-radius: 5px; 

bad indentation for all properties in the rule

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



More information about the webkit-unassigned mailing list