1123829 - (lightsaber) create a color picker web component

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Components, defect)

Description

[David Flanagan [:djf]]

We need a color picker ASAP for the lightsaber project.

Comment 1

[Justin D'Arcangelo [:justindarc]]

GitHub repo: https://github.com/gaia-components/gaia-color-picker

Comment 2

[David Flanagan [:djf]]

Visual design is here: https://mozilla.app.box.com/s/3mu24v068bk3pcaa2oharm4353bfybny

Comment 3

[David Flanagan [:djf]]

Attachment: work in progress screenshot

Patryk,

This is the work-in-progress I have for the circular color picker.  I don't have a working component to try out yet, but I do have code that draws everything you see in this screenshot.

The colors are HSV, which is what I think you had in mind. The colors in the large central ring all have the same value. Hue varies with angle and saturation varies with radius. The outer ring shows colors with the same hue and saturation where only the value differs. The innermost circle is a solid swatch of the selected color.  The small white circle is centered around the current hue and saturation and the small black line points to the currently selected value.

I know that this is not exactly what is in your image, but I want to check that I'm on track or at least close enough for now.

In particular, in your drawing in https://mozilla.app.box.com/s/3mu24v068bk3pcaa2oharm4353bfybny you have an outer ring that seems to be showing about 10 distinct values and inside that you have another ring that seems to be showing shades of gray. I'm not sure what those shades of gray are supposed to be. Are those lightness values and is the idea that this picker would support HSV and HSL colors or something?

Comment 4

[Patryk Adamczyk [:patryk] UX]

Attachment: colourpicker_notes.jpg

David, thanks for this. I made a few notes in the attachment. I think its looking pretty solid so far.

I personally prefer HSV/HSL as its more user friendly, RGB is much less intuitive. 

The gray ring is for selecting only tones, but I don't think the initial design made perfect sense and its not mobile / touch optimized, so I have another solution in the attachment.

+NI Amy as an FYI

Comment 5

[David Flanagan [:djf]]

Attachment: preliminary pull request

Justin: this is a preliminary PR for a color picker component. It is not done, but I hope it is enough for someone to get started using it. Does it look solid enough to you to land?

Fabrice and Etienne: I'm not sure how you plan to use this component. Does this look like it gives you the API that you need? (The API is docmented at the start of the file, so you can just read that and ignore the code, if you want.)

Comment 6

[David Flanagan [:djf]]

Attachment: updated screenshot

This screenshot is closer to what Patryk was asking for, but does not include all of what he wants.

Note that this component is just the circular part of the color picker. If we want a color picker dialog that displays the selected color and its hex values, etc. we can build that from this simpler component.

Comment 7

[[:fabrice] Fabrice Desré]

I defer all the nice pixels to Etienne!

Comment 8

[David Flanagan [:djf]]

I've updated the PR. The component is a lot more featureful now.

Demo is available online at http://djf.net/colortest.html

You have to switch on the web components pref in about:config in your browser to get this to work, of course.

I can't figure out how to get it working in an app on the phone right now, so I don't have anything that Amy can try out.

Comment 9

[David Flanagan [:djf]]

Attachment: screenshot

Comment 10

[David Flanagan [:djf]]

I've run into something that looks like a <canvas> or graphics bug on FxOS. The color wheel displays just fine on desktop but not on the phone. It looks like my clip region is ignored when I've got globalCompositeOperation set to "copy". Trying to create a reduced test case now.

Comment 11

[David Flanagan [:djf]]

Bug 1126055 is the canvas bug that I've run in to where rendering on FxOS and Android differs from desktop.

Until that is resolved, I'm going to have to modify the color picker so that the central color wheel always shows bright colors with value=1. The won't be dimmed as the user adjusts the value in the outer ring. (We could make a case that it should be that way in the first place, of course)

Comment 12

[Etienne Segonzac (:etienne)]

Comment on attachment 8553417 [details] [review]
preliminary pull request

> Fabrice and Etienne: I'm not sure how you plan to use this component. Does
> this look like it gives you the API that you need? 

Yes it does!
Please let me know once this works on a phone and I'll integrate it right away.

Random notes:
- can it be resized during it's lifetime? (not blocking, just wondering)
- can we tweak the borders color (borders between the 2 circles, borders for the cursor)? (although I'm sure the default ones picked by Patryk and Amy will be nice:))

Comment 13

[David Flanagan [:djf]]

(In reply to Etienne Segonzac (:etienne) from comment #12)

> Please let me know once this works on a phone and I'll integrate it right
> away.

This works on the phone. The central disk of colors does not dim and brighten as the user changes the value, until but 1126055 is fixed. But I could see that being considered a feature anyway.
 
> Random notes:
> - can it be resized during it's lifetime? (not blocking, just wondering)

I can't figure out a way for the component to tell when it has been resized, so I've added a resize() method. Call when there is a resize event on the window or when you've changed the size of the component in some other way.  

> - can we tweak the borders color (borders between the 2 circles, borders for
> the cursor)? (although I'm sure the default ones picked by Patryk and Amy
> will be nice:))

Not right now. Some are hardcoded in CSS and other are hardcoded in canvas drawing code. I don't know how CSS templating works, but if we make this component use CSS variables so it can work with templates then at least some of those colors would be customizable, I guess.

See the demo app in the demo/ directory.

Comment 14

[David Flanagan [:djf]]

Wilson: when you have a chance, I'd appreciate your feedback on this component. Note that this is a raw custom element using document.registerElement directly. It does not use your base class, and I'm not sure whether or why I should use that.

Comment 15

[David Flanagan [:djf]]

Comment on attachment 8553417 [details] [review]
preliminary pull request

Amy: I've gone ahead and landed an implementation of this component, with the expectation that you will probably want changes to it.

You can try it out by cloning the repo with:

  https://github.com/gaia-components/gaia-color-picker.git

And then use the WebIDE in FirefoxDeveloperEdition to push the gaia-color-picker/demo directory to the phone.  Or, just load gaia-color-picker/demo/index.html into Firefox and try it out on desktop.

Comment 16

[David Flanagan [:djf]]

Attachment: screenshot of the demo app

Comment 17

[Justin D'Arcangelo [:justindarc]]

(In reply to David Flanagan [:djf] from comment #14)
> Wilson: when you have a chance, I'd appreciate your feedback on this
> component. Note that this is a raw custom element using
> document.registerElement directly. It does not use your base class, and I'm
> not sure whether or why I should use that.

David, the gaia-component base class would allow those themeable colors using CSS vars to be exposed. It overcomes some platform limitations at present that allow those theme colors in the shadow DOM to be set from the light DOM. It should be relatively trivial to migrate from your "vanilla" Web Component to utilize the base class, but we can make that transition later.

Comment 18

[Amy Lee [:amylee] UX]

Comment on attachment 8553417 [details] [review]
preliminary pull request

Including Patryk for UI review.

Comment 19

[Etienne Segonzac (:etienne)]

(In reply to David Flanagan [:djf] from comment #15)
> 
> You can try it out by cloning the repo with:
> 
>   https://github.com/gaia-components/gaia-color-picker.git
> 

Was the correct version merged on github? The demo is not working for me.
(everything's white, no JS error in the console)

Comment 20

[David Flanagan [:djf]]

(In reply to Etienne Segonzac (:etienne) from comment #19)
> (In reply to David Flanagan [:djf] from comment #15)
> > 
> > You can try it out by cloning the repo with:
> > 
> >   https://github.com/gaia-components/gaia-color-picker.git
> > 
> 
> Was the correct version merged on github? The demo is not working for me.
> (everything's white, no JS error in the console)

Were you trying in on desktop? And if so, did you set dom.webcomponents.enabled to true in about:config?

If that is not it, note that  demo/js/gaia-color-picker.js is a symbolic link to ../../gaia-color-picker.js. I've never tried checking a symbolic link into github before, so possibly that broke something?

It was working for me on my mac. I just tried cloning the repo to a linux machine and then pushing the demo/ app to the phone with WebIDE. This was running a nightly build from yesterday. It worked for me.

Comment 21

[Etienne Segonzac (:etienne)]

(In reply to David Flanagan [:djf] from comment #20)
> It was working for me on my mac. I just tried cloning the repo to a linux
> machine and then pushing the demo/ app to the phone with WebIDE. This was
> running a nightly build from yesterday. It worked for me.

That's exactly what I did :/
I'll give it another go!

Comment 22

[Etienne Segonzac (:etienne)]

(In reply to Etienne Segonzac (:etienne) from comment #21)
> (In reply to David Flanagan [:djf] from comment #20)
> > It was working for me on my mac. I just tried cloning the repo to a linux
> > machine and then pushing the demo/ app to the phone with WebIDE. This was
> > running a nightly build from yesterday. It worked for me.
> 
> That's exactly what I did :/
> I'll give it another go!

Don't know which version bower grabbed yesterday, but I just git cloned the repo and the demo is working!
Pretty smooth too :)

Comment 23

[Wilson Page [:wilsonpage]]

Comment on attachment 8553417 [details] [review]
preliminary pull request

REPO STRUCTURE

Could you conform to the repo structure of the other gaia-components. Each repo should have a working /index.html served from a gh-pages branch.

The README.md should link to the demo and provide at minimum install instructions, and if you have time brief API overview.

TESTS

This looks like a very complicated component, that really needs tests. Checkout gaia-header [1] as an example of how to author tests and have them run on commit against Travis.

GAIA-COMPONENT BASE CLASS

You don't have to use the base class, but it's good if you can. The class works around some gaps in the platform (such as shadow-dom css selectors) and provides: a way of keeping attributes and properties in sync, RTL 'dirchange' callbacks, optimizations. It's nice that we can target bugs in one place.

As the platform improves gaia-component.js will become lighter, but it's unlikely to disappear. With web-components APIs being so low-level, I feel we need something like this to make sure we have consistency across all our components. This makes us look more professional and it's helpful for authors and users.

PACKAGE DEFINITION

It's important that you provide a package JSON definition. This should come in the form of bower.json or package.json (but ideally both). At the bare minimum these files should provide a 'name', 'version' and 'main'. Again use gaia-header as a template for this.

VERSIONING

You need to version (semver) your component using git tags whenever anything changes. If you don't users/package-managers won't know when your package has changed, or when you have introduced breaking changes. `bower version patch | minor | major` makes this very easy.

Comment 24

[Wilson Page [:wilsonpage]]

[1] http://github.com/gaia-components/gaia-header

Comment 25

[David Flanagan [:djf]]

Thanks for the detailed to do list, Wilson. 

I've filed bug 1127427 for those things and am closing this one. Note, though, that I will be busy with 2.2 bugs and won't have time right away to make the changes Wilson requests.

Patryk and/or Amy: if you want visual or UX changes to the component, let's file a new bug for those changes.

Comment 26

[Amy Lee [:amylee] UX]

(In reply to David Flanagan [:djf] from comment #25)
> Thanks for the detailed to do list, Wilson. 
> 
> I've filed bug 1127427 for those things and am closing this one. Note,
> though, that I will be busy with 2.2 bugs and won't have time right away to
> make the changes Wilson requests.
> 
> Patryk and/or Amy: if you want visual or UX changes to the component, let's
> file a new bug for those changes.

Hi David, 

I'm working on a list of edits and will post it to a new bug shortly. Thanks