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

[bugzilla-daemon at webkit.org]

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





--- Comment #11 from Brian Grinstead <briangrinstead at gmail.com>  2011-11-21 19:12:33 PST ---
(In reply to comment #3)

I have finished most of this (and the UI) feedback, but am not sure how to proceed with the Color.js changes.

> > Source/WebCore/inspector/front-end/Spectrum.js:82
> > +WebInspector.Spectrum.hsvToRgb = function(h, s, v, a) {
> 
> Seems like this could belong to Color.js
> 
> > Source/WebCore/inspector/front-end/Spectrum.js:116
> > +WebInspector.Spectrum.rgbToHsv = function(r, g, b, a) {
> 
> ditto
> 

I do agree that the Color class should probably have this functionality.  Since HSV colors aren't supported by the css parser it might be confusing if it worked in the web developer ui.  If someone copied an HSV into the value element, for instance, that could be confusing.

> > Source/WebCore/inspector/front-end/StylesSidebarPane.js:1680
> > +                var rgba = (color.rgba || color.rgb).slice(0);
> 
> It sounds like Color class itself is broken. It should maintain single data entry for rgb / rgba.
> 

I was a little leery of making changes to the color class since I didn't know of all the places it was used.  I didn't want to change the default behavior of return values for this patch to prevent messing up existing code.  I don't mind making this change, but I would want some guidance on where to look for problems.


Besides that, I have two issues I want to address before resubmitting a patch:

1. The big issue I am still seeing is that when I add a new property: value; string to an element.  When I open the colorpicker on that element, it seems like it is reinitializing the picker on a minor change event.  This might be because the initialization code is in the updateTitle function.  I have been trying to console.trace() or enter into the debugger to figure out what is going on, but haven't quite gotten it yet.  Any tips on why this might be happening?

2. Also, I want to attach the picker to the _parentPane instead of the valueElement, so that I can put labels inside of the picker (I can't leave it in the valueElement because other code uses valueElement.textContent to determine what CSS to apply).  For some reason, the self._parentPane is sometimes null in updateTitle, so I ended up checking this: 
if (self._parentPane) {
      // initialize colorpicker only if there is a parentPane
      var spectrum = new WebInspector.Spectrum(swatchElement, self._parentPane.bodyElement, rgba);
}
I'm not sure why this code is being called on StylePropertyTreeElement objects that have a null _parentPane.  Again, any tips on why this might be and if what I am doing now should be fine?

You

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