Gradient picker: Small refinement to "Remove Control Point". #37178
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jasmussen
added
[Type] Enhancement
|
Size Change: +175 B (0%) Total Size: 1.11 MB
ℹ️ View Unchanged
|
|
I think the text link looks better and is easier to understand. The icon is slightly mysterious (what am I throwing in the trash?) and also visually unbalanced. I think the label could be updated to something like "Remove color" and would be pretty good. |
|
Do you think removing a control point deserves so much prominence? |
7 tasks
|
Its already pretty hidden inside the popover. Once I've indicated I want to adjust the control point, then I think removing it should be obvious. I'm not sure making it a trash icon necessarily reduces it's prominence; It does take up less space, but its also more mysterious and unclear. |
|
I agree with @shaunandrews about it feeling unbalanced in the corner. Having a little bit more prominence by centering I think would be fine, or shortening the text to read "Remove color" would also be okay. Anyway, the fix for hiding the label was a quick one, so the PR is up #37186 I see there's also a bug with the control point jumping towards the center when you click on it in your GIF, so I might give that a look as well if it happens to be a quick fix. |
|
I guess this needs a more holistic design then, though I still feel like the titlecase and the centered + underlined style for the button is awkward. Thanks for the feedback, and thanks Alex for the separate PR for the multiple controls, that's a big step on its own. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The flow for removing gradient control points is currently a bit challenged:
Specificlally, the "Remove Control Point" button has undue prominence, is a link button, uses titlecase, and doesn't really sit in a well-balanced part of the interface.
This PR is what I'd consider a very small bandaid to reduce the prominence a little bit, without making big changes. It just makes it a small trash button:
I don't really like the placement or flow, or the icon. To an extent I would think it plenty sufficient for the Delete key to remove the point, as it's such an edgecase that it really should have little prominence in the UI. But as a temporary holdover, this seems like a small win.
I also noticed by the way that you can endlessly remove control points. It seems like we should show the "remove control point" button only if there are more than two control points, since the gradient isn't actually functional with less than that:
@ajlende is this something you can help me with?
Checklist:
*.native.jsfiles for terms that need renaming or removal).