[web] Attempt to fix `long-right-angle-arrow`
ClosedPublic
Actions

Authored by atul on Apr 25 2022, 1:51 PM.

Details

Reviewers

• jacek
tomek
• benschac

Commits

rCOMM8086f93dbbd4: [web] Attempt to fix `long-right-angle-arrow`

Summary

Attempted to fix the long-right-angle-arrow SVG to avoid the weird artifacts that were showing up in Safari.

Test Plan

This seemed to work for me, would be good for @benschac to patch this and take another look.

Diff Detail

Repository

rCOMM Comm

Branch

svg-fix (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Apr 25 2022, 1:51 PM

Herald added subscribers: • abosh, • karol, • adrian, ashoat. · View Herald TranscriptApr 25 2022, 1:51 PM

atul edited the summary of this revision. (Show Details)Apr 25 2022, 1:53 PM

atul edited the test plan for this revision. (Show Details)

Harbormaster completed remote builds in B8531: Diff 11890.Apr 25 2022, 1:56 PM

atul requested review of this revision.Apr 25 2022, 1:56 PM

This looks alright for me. What did you end up doing to actually fix the problem? Did you edit the SVG in Illistrator?

In D3843#106647, @benschac wrote:

This looks alright for me. What did you end up doing to actually fix the problem? Did you edit the SVG in Illistrator?

Removed fill (as we discussed) but there were still some remaining artifacts (as we saw).

Looked at the vector on Figma and saw that the arrow was a combination of 4 separate path which intersected in a weird way. Flattened them into a single path in Sketch and exported as SVG. Could have just as easily done it in Figma/Illustrator/etc. Just went with Sketch out of convenience.

The IcoMoon docs have a section on preparing the source assets that mentions flattening paths/converting strokes to fills/etc

great, let's favor this diff instead of D3730, we should still add color to the component now that the fill is no longer rgb(128, 128 ,128) via the SWMIcon props.

• benschac mentioned this in D3730: [web] [fix] [ENG-1005] arrows visually broken on safari.Apr 26 2022, 7:09 AM

• benschac accepted this revision.Apr 26 2022, 7:13 AM

This revision is now accepted and ready to land.Apr 26 2022, 7:13 AM

atul added a reviewer: ashoat.Apr 26 2022, 7:28 AM

This revision now requires review to proceed.Apr 26 2022, 7:28 AM

It might be a good idea to check this on multiple browsers

web/icons/selection.json
937	I don't know svg, but do we need to have an empty object here? Maybe we can skip this attribute entirely?

atul removed a reviewer: ashoat.Apr 26 2022, 9:35 AM

This revision is now accepted and ready to land.Apr 26 2022, 9:35 AM

atul added inline comments.Apr 26 2022, 9:38 AM

web/icons/selection.json
937	The IcoMoon tool that we're using for these icons requires that we include the `attrs` property. But, it looks like we can leave it as an empty array and can exclude the empty "object" inside. Updating this diff to clean that up.