Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add nudge to include name for Custom Color Palette #36473

Closed
annezazu opened this issue Nov 14, 2021 · 7 comments · Fixed by #36940
Closed

Add nudge to include name for Custom Color Palette #36473

annezazu opened this issue Nov 14, 2021 · 7 comments · Fixed by #36940
Assignees
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Design Feedback Needs general design feedback. [Type] Enhancement A suggestion for improvement.

Comments

@annezazu
Copy link
Contributor

annezazu commented Nov 14, 2021

What problem does this address?

This is pulled from the eleventh call for testing for the FSE Outreach Program:

I actually missed that I needed to add a name to the color at all 😅 Since there’s no placeholder text in the area where the color name should be added, I completely overlooked it and assumed I would just add the color, click Done, and voilà! However, it looks like not adding a color name at all means the colors won’t get saved. Adding some directive placeholder text next to the colors – or even an error message after clicking Done – might have helped me move past that.

& another person:

Just like @evarlese observed (and thanks for the precious notes!), the way to deal with the Custom Color Palette is not straight forward, at all. She already explained it really well, so adding my +1 also to the proposed solutions. Kudos Erica!

Custom.Colors.testing.mp4

What is your proposed solution?

Rather than resetting the custom color and not saving it, add in a prompt or warning to have someone include a name for the color. Perhaps it could be as simple as highlighting the field if it's empty or adding in placeholder text for where the name needs to go:

Screen Shot 2021-11-14 at 2 07 22 PM

To make it less jarring, could just add a weighted border that's black to point it out more as a field:

Screen Shot 2021-11-14 at 2 08 17 PM

I'll leave it to designers :D

@annezazu annezazu added [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Nov 14, 2021
@annezazu annezazu added Needs Design Needs design efforts. and removed Needs Design Feedback Needs general design feedback. labels Nov 16, 2021
@noisysocks
Copy link
Member

noisysocks commented Nov 17, 2021

We should also add validation to this field. Names containing a space don't work for me when I use the custom colour. (Might be browser specific, I'm testing with Safari.)

@noisysocks noisysocks added this to 📥 To do in WordPress 5.9 Must-Haves via automation Nov 17, 2021
@javierarce javierarce self-assigned this Nov 17, 2021
@javierarce
Copy link
Contributor

Rather than resetting the custom color and not saving it, add in a prompt or warning to have someone include a name for the color. Perhaps it could be as simple as highlighting the field if it's empty or adding in placeholder text for where the name needs to go

Depending on how much time we have to fix this UI, I can propose different solutions. Here's the quickest one:

Similar to what @annezazu proposes, I suggest adding a temporary color name using "color" plus the number of items on the list.

image

The reason for this suggestion is that right now, when the user clicks the + button to add a new color, we immediately show the popover, and then we close it when they click outside. At the same time, we hide the input field, so I fear that some people will forget to change the "enter name" prompt.

Another important thing I would add right away is the hover state for the input fields when they aren't active:

Hover.mp4

@javierarce javierarce moved this from 📥 To do to 🏗️ In progress in WordPress 5.9 Must-Haves Nov 17, 2021
@javierarce javierarce added the Needs Design Feedback Needs general design feedback. label Nov 17, 2021
@colorful-tones
Copy link
Member

There may be some overlap here with Color naming conflict for theme variables #36360. Especially with mention of color name validation.

@javierarce javierarce moved this from 🏗️ In progress to 👀 Needs review in WordPress 5.9 Must-Haves Nov 17, 2021
@javierarce javierarce removed the Needs Design Needs design efforts. label Nov 17, 2021
@Mamaduka Mamaduka moved this from 👀 Needs review to 📥 To do in WordPress 5.9 Must-Haves Nov 18, 2021
@richtabor
Copy link
Member

I like the idea of having the intiial label for a color being "Color $count", so the user doesn't have to think of/provide a label, and it is required.

@jiteshdhamaniya
Copy link
Contributor

I'd just create a PR for it.

@ramonjd
Copy link
Member

ramonjd commented Nov 29, 2021

I've started a PR over at #36940

It only adds the default name for new colors so far, but I can look at validation and onHover effect mentioned in #36473 (comment) as well.

WordPress 5.9 Must-Haves automation moved this from 📥 To do to ✅ Done Dec 1, 2021
@ramonjd
Copy link
Member

ramonjd commented Dec 1, 2021

Another important thing I would add right away is the hover state for the input fields when they aren't active:

Getting the hover state width right is difficult since the input width, when editing, is constrained by the minus button.

The minus button isn't rendered when the item isn't in editable mode.

Getting it this far required a minimal change however:

Kapture 2021-12-01 at 13 49 47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Design Feedback Needs general design feedback. [Type] Enhancement A suggestion for improvement.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

7 participants