Implement color palettes in the color widget

NEW
Unassigned

Status

()

Firefox
Developer Tools: CSS Rules Inspector
P3
enhancement
10 months ago
2 months ago

People

(Reporter: gl, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

10 months ago
This bug is to implement a simple color palette in the color widget as seen in https://projects.invisionapp.com/share/9G5R8XCYZ#/screens/143217180.
(Reporter)

Comment 1

10 months ago
Patrick, can you advise on the scope of this initial basic color palette and recommend how to go about it.
Flags: needinfo?(pbrosset)
(Reporter)

Updated

10 months ago
Blocks: 1277352

Comment 2

10 months ago
I think this might need to become a meta bug instead and have several child bugs.
The design spec proposes several types of palettes:
- named colors
- page colors
- recently picked colors
- firefox palette
- create your own palette

Each of these will require substantially different code changes and I think they would be better handled as separate bugs.

Did you have a priority order in mind already? My suggestion would be to work on the page colors first. This is, to me, the most useful.
Named colors are nice because they give you a quick list of colors you could try without having to try your luck with the color wheel, but I think a good first step would be to extract page colors.
Recently picked colors sound really useful too to be honest, but I would do that later too.
Firefox palette, not sure what this is, not sure how useful this would be to web developers outside of mozilla.
And last, create your own palette will be a lot more complicated to implement and might be used less, so I'd keep this for later too.

So, let's talk about extracting page colors first:

I found an old bug that I had been working on: bug 1136196. I even has a lot of discussion and a patch!
Please read through the bug as it will give you more context about what it is we can do.
In fact, maybe we should reuse that bug and make a child of this meta bug here.

I think I was over-complicating things in the patch though, I don't think we need any of the color swapping mechanism in there.
Also the code is too old and won't be rebase-able.
However there may be some code you can reuse. Look at the patch and search for colorExtractors.
This contains a set of functions that extract colors from a page. There were 3 main solutions discussed: extracting colors from stylesheets, or from the DOM (computed-style), or from an screenshot of the page. Pros and Cons of each are discussed in that bug.

I would suggest starting with the stylesheet approach first. Iterating over stylesheets/rules/properties is fairly straightforward, but the hard part will be to sort colors so the palette is organized in some way, and possibly de-duplicating colors. This might be hard because I think we might want to even de-dupe colors that are very close to each other. When you start having many stylesheets and rules on your site, it's easy to lose track of how many grays you have for example. You may start having several slightly different grays when, really, you should just have one.
This should be yet another bug though, we can start by just getting *all* colors.

The code that will extract colors will need to run on the server, in an actor. I suggest creating a new actor: ColorActor which will be responsible for anything color-related. I don't really see this being a good fit for the Inspector actor.
Maybe this could live in the StyleSheetActor... but if we're adding other color extraction strategies later, it might not make sense to have it in that actor.
So I really think that have a new ColorActor is the best approach.

Once you have that actor in place (and some tests for it), you'll need some front-end in the colorpicker. We will need to be careful how we request these colors. This can potentially take time (if there are many stylesheets and rules), and we don't want to do this all the time. We will also need to be backward compatible, since older backends won't have this actor, the palette UI should be hidden for them.

The 2 other methods for extracting colors are:
- from the DOM, using computed styles: iterate over all nodes on the page, getting their computed-style each time, and then iterating over all of the style's properties, getting colors from there.
- from a screenshot: draw the whole window into a <canvas>, and then iterate over each and every pixel in it, getting its color. I don't like that because it also gets all individual colors from gradients, and from images.

About named colors.

There is a finite list of colors with names, this list almost never changes, so it's safe to hard-code it somewhere.
In fact, we already have it in devtools\shared\css\color-db.js
This file was generated from the actual list of color names in Gecko, so don't change it. We have a test that checks if that file is still correct.
So, you could use that to generate the palette. No need for a server-side actor for this even, because this file is in /devtools/shared/ so it can be used from the client-side directly!

Recently picked colors.

This sounds like a rather simple (client-side-only) thing we could do too. Anytime a color is picked in the colorpicker, store it in an array somewhere, and populate this palette with these colors.
Now, should this palette also contain colors entered manually in the rule-view, without the colorpicker? Not sure.
Should if persist when devtools/firefox is closed? If so, should it be persisted per domain name?
Those are harder questions to answer, so I'd suggest taking a step approach, filing different bugs, and just starting with the simplest thing: just storing in an array in memory, not persisting anything.

Creating your own palette.

This is more complex UI-wise, because you almost need to open a colorpicker inside the colorpicker to let a user enter a new color for their palette.
Although this feature seems really cool, I would really keep it for much later because of the complexity and also because I suspect it won't be used much. 
The same persistence questions than before also apply here.
Flags: needinfo?(pbrosset)
(Reporter)

Updated

9 months ago
Assignee: lockhart → nobody
Status: ASSIGNED → NEW

Updated

2 months ago
Severity: normal → enhancement

Updated

2 months ago
Duplicate of this bug: 1420195
You need to log in before you can comment on or make changes to this bug.