Page MenuHomePhabricator

[web] Remove default stroke for `wrench` icon
ClosedPublic

Authored by atul on Mar 7 2022, 8:05 PM.
Tags
None
Referenced Files
F3301141: D3362.diff
Mon, Nov 18, 12:30 AM
Unknown Object (File)
Sat, Nov 2, 8:55 AM
Unknown Object (File)
Sat, Nov 2, 8:55 AM
Unknown Object (File)
Sat, Nov 2, 8:55 AM
Unknown Object (File)
Sat, Nov 2, 8:53 AM
Unknown Object (File)
Oct 16 2024, 3:52 AM
Unknown Object (File)
Oct 16 2024, 3:52 AM
Unknown Object (File)
Oct 16 2024, 3:52 AM

Details

Summary

Without this change, we aren't able to change the color of the wrench icon, which makes styling it on hover, etc impossible.

There was a default stroke color for the wrench icon that couldn't be overridden via CSS... so removing it manually for now to unblock.

We converted stroke/path to object/shape before importing to IcoMoon when we included the icons on native, but I guess didn't for web?

Open to any/all alternative approaches, this is all I could get to work.

Test Plan

NA, able to style the wrench icon

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul requested review of this revision.Mar 7 2022, 8:11 PM
tomek requested changes to this revision.Mar 8 2022, 8:20 AM

Does this issue affects other icons we have?

Is this icon used in other places where we need to check if it is still rendered correctly?

My guess is that it should be possible to style this by using stroke CSS property.

This revision now requires changes to proceed.Mar 8 2022, 8:20 AM

Does this issue affects other icons we have?

Yeah, there are a few other icons with a stroke color that's "hard-coded." I was going to "fix" them as welI, but wanted to limit the scope of this diff since @benschac is working on getting all the icons figured out.

Is this icon used in other places where we need to check if it is still rendered correctly?

Did a search for "wrench" in the project repo and didn't find any other occurrences other than in app-switcher.react.js

My guess is that it should be possible to style this by using stroke CSS property.

I tried doing that and it worked for the other icons in the AppSwitcher, but it didn't work for this one without removing the hard-coded value.

There's probably a way to get things to work without making this change, but I spent ~15 minutes and couldn't figure out anything obvious.

I figured I wouldn't spend too much time investigating since @benschac is figuring out the icons, but I can definitely take another look if this continues to be an issue. As part of that work, I think converting [paths/strokes] to [shapes/objects/outline] would also generally improve the fidelity of the icons we're using.

atul requested review of this revision.Mar 8 2022, 8:35 AM

Ok, it makes sense to introduce this workaround now. Do you know if the other icons also have hardcoded stroke?

This revision is now accepted and ready to land.Mar 8 2022, 8:41 AM

Do you know if the other icons also have hardcoded stroke?

This is the only one with a hardcoded stroke, but there are others with hardcoded fills that I imagine will also have issues.

I'll sync up with @benschac sometime today on fixing up the icons