Page MenuHomePhabricator

[web] Set `stroke: none` for `CommIcon` SVGs
ClosedPublic

Authored by atul on Aug 17 2022, 11:42 AM.
Tags
None
Referenced Files
F3360737: D4864.diff
Sun, Nov 24, 2:10 PM
Unknown Object (File)
Fri, Nov 22, 10:26 AM
Unknown Object (File)
Fri, Nov 22, 5:45 AM
Unknown Object (File)
Sun, Nov 3, 2:17 PM
Unknown Object (File)
Oct 19 2024, 1:23 AM
Unknown Object (File)
Oct 19 2024, 1:23 AM
Unknown Object (File)
Oct 19 2024, 1:23 AM
Unknown Object (File)
Oct 19 2024, 1:23 AM
Subscribers

Details

Summary

Noticed that the SVG icons we're importing via <IcomoonReact> weren't quite as smooth as they could be, specifically noticed some artifacts on the "corners" of the path of the reply-filled icon.

Screen Shot 2022-08-17 at 2.08.39 PM.png (208×412 px, 34 KB)

I tried various approaches (from StackOverflow, blogs, etc) to try to determine what was causing the artifacts and how to resolve the issue (eg setting shape-rendering explicitly).

In experimenting with the icon using the browser developer tools, I noticed that setting stroke to none caused the artifacts to disappear. This was strange because the source SVG for reply-filled we're passing into the <IcomoonReact> component is a single path with a fill and no stroke. So I tried to figure out where the stroke was coming from.

I clicked into node_modules/react-icomoon/src/index.tsx (https://github.com/aykutkardas/react-icomoon/blob/807a42cf61c8e26c078c5818ba0736afc003032a/src/index.tsx#L60) to see what was going on and found that <IcomoonReact> prepares a container for the SVG to "live inside" and sets various SVG properties. Specifically, they set stroke to currentColor. Even though there's no stroke width or whatever, this leads to some visual weirdness when rendered in Safari/Chrome.

Screen Shot 2022-08-17 at 2.36.30 PM.png (260×612 px, 38 KB)

To verify that IcomoonReact setting stroke was the issue, I went into the library's dist folder and tried commenting the line out. After reloading the web app I confirmed that the artifacts no longer appeared.

Screen Shot 2022-08-17 at 2.33.30 PM.png (260×612 px, 33 KB)

Screen Shot 2022-08-17 at 2.10.22 PM.png (208×412 px, 10 KB)

This diff explicitly sets stroke to none in the CommIcon component which passes the style down to IcomoonReact to override the stroke set in the inner component. This change gets rid of the artifacts and the icons look as expected.

Test Plan

Things look as expected on Safari/Chrome/Firefox

Diff Detail

Repository
rCOMM Comm
Branch
arcpatch-D4864 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul edited the summary of this revision. (Show Details)
atul requested review of this revision.Aug 17 2022, 11:52 AM
This revision is now accepted and ready to land.Aug 17 2022, 1:04 PM
This revision was landed with ongoing or failed builds.Aug 18 2022, 8:23 AM
This revision was automatically updated to reflect the committed changes.