[color-scale] editable prop (MVP)

[DmitrySharabin]

Live preview: https://deploy-preview-167--color-elements.netlify.app/src/color-scale/#the-editable-attribute

[netlify]

✅ Deploy Preview for color-elements ready!

Name	Link
🔨 Latest commit	49fb4d6
🔍 Latest deploy log	https://app.netlify.com/sites/color-elements/deploys/67601b047ce21a0008f6bee0
😎 Deploy Preview	https://deploy-preview-167--color-elements.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[LeaVerou]

See comments. Also I don't see anything about what keywords are used in editable

[LeaVerou]

Some (possibly non-exhaustive) comments:

 

• High level: I would have expected this to involve a lot more changes in <color-swatch>. Why is it trying to do everything in <color-scale>? A lot of this functionality is useful for individual swatches too. I'd expect mainly the list stuff to be in <color-scale>. Also, [color-swatch] Decouple color and label concepts #107 seems to be causing issues here too (see below).

Delete button:

• Remove button should be a part
• I'd use "delete" rather than "remove" consistently
• The delete button should only appear on interaction, otherwise it adds too much visual noise
• The current styling is more "not allowed" rather than "delete"

Docs:

• Don't show the edge cases before the simple syntax. start from simple, then complex. I'd show the boolean editable first, and then the values to customize the behavior.

Other:

• Why are colors displayed twice in

- Good call on making interpolated colors not editable! Post-MVP, we could make the *number* of stops editable. - I would style the add button like an outlined swatch with a light gray border and a big + - The only values I see for `editable` are `name` and `color`. How do I specify whether the list itself is editable and how? (add, delete, reorder, possibly with a `list` shorthand for all three) - I wonder if a better UI for editing the color/name is a pencil icon next to it which makes it editable then. Or some way to choose. - Since UI is likely expected to reactively change the value of `edit` to toggle editing, I wonder if we should call it `edit` rather than `editable`. And we should be consistent across the library.

Food for thought:

• It would be good to start thinking how we can support the common need of modifying colors in a scale with a more elaborate UI, such as the one by our <color-picker> or even a third-party one, e.g. like

[LeaVerou]

I know the change was my idea, but seeing it now, I think editable was a better name 🙈

Also, could we add an editable attribute on color swatches and use that rather than encoding color-swatch logic here?

[DmitrySharabin]

I know the change was my idea, but seeing it now, I think editable was a better name 🙈

No worries—it's an easy change. I see nothing wrong with trying things out and reverting to one of the previous options if the new ones fail.

Also, could we add an editable attribute on color swatches and use that rather than encoding color-swatch logic here?

Sure. It was exactly the next step I was going to take. I just keep the things around so I don't lose progress.

[DmitrySharabin]

I think the PR is ready for another iteration. Now, it does even more than before:

• <color-swatch> — https://deploy-preview-167--color-elements.netlify.app/src/color-swatch/#the-editable-attribute
• <color-scale> — https://deploy-preview-167--color-elements.netlify.app/src/color-scale/#the-editable-attribute

• Why are colors displayed twice in

The first are the color names, and the second are the colors themselves. Since we are editing colors here, the names are still visible. Anyway, I updated the example to be less confusing.

I addressed most of your feedback. Thank you for it (as always, it's highly valuable). 🙏

Still to implement: reordering colors, changing the number of "stops," enabling edit mode with a pencil button next to the color label, and changing colors with a popup. Probably, next iteration?

@LeaVerou, could you please give the PR another shot?

[LeaVerou]

I have not yet properly reviewed it, just wanted to share some rendering glitches on Safari iOS:

also, the + button needs and outline so that it looks a bit more visually similar to the swatches

can we make the name editing UI a bit less prominent than the color editing UI? When both are editable, there is currently nothing to guide the eye, and I think when both are editable, the color is edited a lot more

[LeaVerou]

I would suggest instead checking if it's focused (via document.activeElement IIRC). It's entirely possible that it's editable but nobody is interacting with it right now, in which case re-rendering would be fine, right?
Though re-rendering seems a very crude reaction to computedColors changing, when does this actually happen? Is it when the entire array if overwritten? In that case I guess it makes sense…

[DmitrySharabin]

I would suggest instead checking if it's focused (via document.activeElement IIRC). It's entirely possible that it's editable but nobody is interacting with it right now, in which case re-rendering would be fine, right?

You are absolutely right. Thank you for the idea. It works brilliantly.

Though re-rendering seems a very crude reaction to computedColors changing, when does this actually happen? Is it when the entire array if overwritten?

It does happen when this.colors is overwritten; you are right.

[LeaVerou]

Need to review this properly, but a couple things that jumped at me from a very cursory look:

• buttons need cursor: pointer.
• the color-swatch docs still shows editable as slotting an input, rather than using the editable attribute.
• this icon is a bit too complex. A pencil icon would suffice. And we should probably use FA icons :)
• This is too visually complex: I would do a simple icon with the right text color (which we know) and no background until it's hovered. Once hovered, then it could have a single background (e.g. red with a white icon)
• I would rather avoid accent colors that may not work well with the host page if we don't need an accent color. I.e. I'd make this gray On hover it could preview what the default color is. It would also be nice to show "Add color" in the swatch name area when you hover.

[LeaVerou]

Still need to review the code, but left some comments about the design/docs.