Swatch picker

[mttshw]

No description provided.

[keithamus]

Looking great! Some notes:

[keithamus]

This is looking really good but I'd recommend some refinements.

One general comment is that there's a mix of whitespace. All of these files should probably use tabs for indentation, perhaps we should install prettier?

Also index.html doesn't link to this page at all, so that would be really nice!

[keithamus]

Tiny nit but you could get away with just < here and others.

Suggested change

	<open-swatch-picker></open-swatch-picker>
	<open-swatch-picker></open-swatch-picker>

[keithamus]

Having this guard prevents the picker from picking the same color twice. STR:

1. Open the page
2. Click "Choose color"
3. Click on the first button
4. Observe dialog is closed, button is populated.
5. Click the button to open the dialog again
6. Click the first button (i.e. same button as first choice)
7. Observe the dialog does not close. Only choice is to pick another color or press Escape.

[keithamus]

Perhaps output-label="" is a more ergonomic way to handle this? As in <open-swatch-picker output-label="">

Perhaps we can do more interest things with a string attribute later on?

(This is non blocking, just a note).

[keithamus]

This dialog has no label, making it a little tricky for screen readers to navigate. Also main is redundant. Some options:

1. Add an aria-label, offers little customisation of the label but also the simplest:

Suggested change

	</dialog>
	<dialog closedby="any" id="select-color-dialog" aria-label="Color Picker">
	${scales.map(scale=>makeScale(scale)).join('')}
	</dialog>

2. Add a named slot so consumers can customise:

Suggested change

	</dialog>
	<dialog closedby="any" id="select-color-dialog">
	<slot name="header" part="dialog-header"><h1>Color picker</h1></slot>
	${scales.map(scale=>makeScale(scale)).join('')}
	</dialog>

[keithamus]

This has no validation which is a little awkward. I'd prefer something like:

Suggested change

	return this.getAttribute('output-type');
	const type = this.getAttribute('output-type');
	if (type == 'hex' || type == 'rgb' || type == 'oklch' || type == 'var') return type;
	return 'var';

This way you can check in JS if something went wrong by setting the value and reading it back.

[keithamus]

This should also have a setter:

Suggested change

	get outputType() {
	set outputType(type) {
	this.setAttribute('output-type', type);
	}
	get outputType() {

[keithamus]

Having var(--emerald-6) is nice but perhaps it's also nice to just get the ident without var(), so you can for e.g. make your own var(--emerald-6, var(--fallback)) or similar.

So perhaps a new output type of 'ident' would be useful here.

This is non blocking.

[keithamus]

This could be a getter, and it probably should be private:

Suggested change

	dialog = null;
	get #dialog() { return this.shadowRoot.querySelector('dialog') }

[keithamus]

label is not reactive, so if I change it, the shadowRoot is not updated. I'd recommend something like:

export class OpenSwatchPicker extends HTMLElement {
	static observedAttributes = ['label']

	//...

	#updateValue() {
		this.shadowRoot
			.querySelector('span[part="label"]')
			.textContent = this.getAttribute('label') || 'Choose Color';
 }

	attributeChangedCallback(name, oldValue, newValue) {
		if (name == 'value') this.#updateValue()
	}

	connectedCallback() {
		//...
 		this.#updateValue()
		//...
	}
}

The same is also likely true for things like output-type, which might have some nice emergent properties if that was reactive.

[keithamus]

Nit, this is probably as readable, but either way is fine:

Suggested change

	const swatchStyle = getComputedStyle(e.target);
	const swatchColor = swatchStyle.backgroundColor;
	this.oklchValue = swatchColor;
	this.oklchValue = getComputedStyle(e.target).backgroundColor;

[keithamus]

Might be nice to handle inputs too:

Suggested change

	for (const el of this.getRootNode().querySelectorAll('input[open-swatch-picker],output[for]')) {
	for (const el of this.getRootNode().querySelectorAll('input[open-swatch-picker],input[for],output[for]')) {