Open matiasbenedetto opened 17 hours ago
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot
label.
If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
Co-authored-by: matiasbenedetto <mmaattiiaass@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
Co-authored-by: jasmussen <joen@git.wordpress.org>
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.
Size Change: 0 B
Total Size: 1.82 MB
Flaky tests detected in 96cf87d8699abbdd4985a68a2a8d1c1b756f2d71. Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.
đ Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11955844505 đ Reported issues:
/test/e2e/specs/editor/blocks/navigation-frontend-interactivity.spec.js
Thanks for the PR, thanks for the ping! I ran storybook, and sure enough, there's a solid new icon.
However, the use cases outlined in the initial comment gives me a bit of pause, because there are some do's and dont's that we need to be careful of, for this particular utility icon. Notably, the icon should always have a stroke, unless it's black. That is part of the morphology of the icons. I recognize there's a use case of colorizing the stroke of the icon according to any border-color set, it's not actually clear to me that we want to do that. But entertaining the thought, the following could be okay:
Note here how there's always a stroke, and that stroke is on the inside. This is what I see in the original examples, the stroke being applied on the outside, and some cases of showing the icon in just a single color, without any of the black color:
Stacked to illustrate inner vs. outer strokes:
The black color of the icon serves a contrast purpose. It is colored according to the UI to ensure there's always at least 3:1 contrast with the surroundings.
For that same reason, I'm ultimately hesitant about adding a fully filled icon like this, at all, because it's not clear you should ever use it like that. The function is only ever designed to show a single color, like so:
I understand that the above is a bit tricky to achieve with how the icon is structured at the moment. Perhaps there's a separate utility component created, that is not part of the library, that has things separated out more cleanly?
@matiasbenedetto might be able to answer better than I but this PR aimed to make the icon more flexible than our real world use case in https://github.com/WordPress/gutenberg/issues/66465#issuecomment-2439960078 & https://github.com/WordPress/gutenberg/pull/67140 requires.
Notably, the icon should always have a stroke, unless it's black
If this icon did always have a stroke would that make it suitable for inclusion in the icon library?
The function is only ever designed to show a single color, like so:
I think this aligns exactly with the intended use in #67140.
If this icon did always have a stroke would that make it suitable for inclusion in the icon library?
Certainly.
If I can include a color
icon and not do anything else, and I get the outlined version with a white background, it's a candate. If I can also provide it a second color that only affects the white background inside the stroke, that's fine too from my perspective. So to be clear I don't mean to block anything from landing in the icons package, rather offer opportunities as far as what's best on the dev side, i.e. if you need additional features in order to colorize the inner fill of the droplet, we could have the inert non-functional icon, and a duplicate that exists just for when the innards need color.
Was that clear? The thing about landing in the icons library means we also add it to the Figma WordPress Design System, where people use the icons for mockups. If there's a utility icon there that's never meant to be used on its own, it probably shouldn't be part of the library.
What?
Adds a filled version of the 'color' icon, 'colorFilled'.
Why?
To be able to implement the UI proposed in https://github.com/WordPress/gutenberg/issues/66465#issuecomment-2439960078 for https://github.com/WordPress/gutenberg/issues/66465
How?
Adds a new icon to the icons library. This is a slightly modified version of the SVG code proposed by @aaronrobertshaw in https://github.com/WordPress/gutenberg/pull/67140#pullrequestreview-2447221592 props to him. I removed some unwanted or hardcore properties to make the SVG code looks like the rest of the filled icons in the package.
Testing Instructions
Import the 'colorFilled' from the
@wordpress/icons
package and observe how it looks. Try to set a fill color and a stroke color too.Screenshots or screencast
Default:
With custom fill color set:
With custom fill and stroke color set: