-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Conversation
I love the shape of the new dev-icon. You can make things standout with color, shape, movement and contrast. Colouring background leaves us no further options for the future.
For now, just leave the background color black (grey) to keep future extensions open. |
Red (#d94a38) doesn't have the same saturation and lightness as the React blue (#61dafb). Red with the same saturation and lightness is #fb6161, so should look better (see colorize). |
I mainly objected in favour of the the red bug and against the red background as it leaves no options for other (future) changes. As shown here, it is either debug or outdated, but not both. I do agree that a mix of colours is probably not going to work for outdated version, too confusing. A warning signs has different associations for me though. Suggestion: Put a yellow label with black text at the bottom with just "OLD" inside. The word is short enough to be visible in small resolutions. |
It seems like using SVG directly won't work well. It isn't supported in manifest, and while it is supported when calling from code, Chrome doesn't take DPI into account so it looks blurry on Retina: There is a hacky workaround involving drawing SVG into a canvas, but it seems like the easier way to solve this is just to create a 32 PNG icon that looks crisp on Retina: The docs don’t make it clear, but apparently providing 32 version of each icon is the way to go (for Retina at least). |
Interestingly, downsizing 128 icon with Preview app gave me a sharper image than generating 32 from SVG directly. |
Ah! I didn't realize that. I'll add 32x versions momentarily. |
Well, I mean, yeah- they'll be as crisp as Photoshop can make them. 😁 |
…e and outdated React versions (neither of which are yet used).
Okay! Just squashed all of the churn-commits and re-pushed:
The team voted for the black bug on red background variant of the development-mode icon but I did lighten the shade of red slightly to address contrast concerns mentioned previously. Anything else needed for this PR? 😄 |
Would be nice to make popup versions that explain each of the icon. |
Definitely! I was hoping to address that with follow-up PRs that actually plug the new icons in. Edit for clarity: As of this PR, only the new higher-res "disabled" and "enabled" (production mode) icons are actually used. Same behavior as before, just better icons. I'm happy to submit follow-up PRs that plug the outdated and dev-mode icons in, and I'll add the new popup versions along with those PRs. |
Sure, I could do that. Would increase the file size of the PNGs slightly b'c of transparency but probably wouldn't be be substantial. |
Yeah, that's something to be a bit wary of. I'll need to round in such a way that Chrome's rounding doesn't leave a gap. (I'd prefer to avoid using different rounded-vs-not-rounded backgrounds for different sizes. It gets difficult to manage.) |
PS Why can't Chrome just round in both places and make my life easier |
I think Chrome only rounds the default icon. It doesn't appear to round ours, so we're free to round it ourselves. |
Looks like if we round our own corners (even for the main image) Chrome won't auto-round. I guess that's to avoid things like I was worried about with mismatching corner radiuses. |
lgtm |
Oh snap one sec |
Can you also verify the contrast is as high as our favicon on React website? |
Okay, let's go with this for now! Thanks. |
Cool. Thanks for the feedback, Dan! |
Anything holding off from merge? |
Nope! gogogo 😁 |
I just implemented the detection btw. Will send a PR shortly. |
Relates to #534
Here's a set of higher-resolution icons for devtools, including a few new icon states that we can add support for in future PRs.
Proposed icons