[web] Clean up `NotificationsModal` SVG assets and remove `SWMansionIcon` dependency
ClosedPublic
Actions

Authored by atul on May 15 2022, 7:16 AM.

Details

Reviewers

• jacek
tomek
• benschac

Commits

rCOMMd2a3eecf5dca: [web] Clean up `NotificationsModal` SVG assets and remove `SWMansionIcon`…

Summary

Linear issue: https://linear.app/comm/issue/ENG-1141/use-svg-instead-of-swmansionicon-in-notificationsmodal

We were previously including the SVG illustrations in NotificationsModal via SWMansionIcon which requires going through the whole "IcoMoon" process. As noted in the Linear issue, the illustrations didn't look great (especially on Safari).

Exported the icons from Figma as SVG, cleaned up the vectors in Sketch, re-exported the cleaned-up illustrations as SVG, uploaded to S3, and replaced SWMansionIcon with <img> in NotificationsModal.

Before:

Screen Shot 2022-05-15 at 10.09.18 AM.png (1×844 px, 101 KB)

After:

Screen Shot 2022-05-15 at 10.09.04 AM.png (1×844 px, 100 KB)

Test Plan

Ensured that the illustrations looked as expected on Safari, Chrome, and Firefox.

Diff Detail

Repository

rCOMM Comm

Branch

master

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.May 15 2022, 7:16 AM

Herald added subscribers: • abosh, • adrian, ashoat. · View Herald TranscriptMay 15 2022, 7:16 AM

atul published this revision for review.May 15 2022, 7:17 AM

atul edited the summary of this revision. (Show Details)

Harbormaster failed remote builds in B9116: Diff 12681!May 15 2022, 7:19 AM

rebase and remove the services CI testing diff from stack

Harbormaster completed remote builds in B9117: Diff 12682.May 15 2022, 7:25 AM

tomek accepted this revision.May 17 2022, 5:56 AM

tomek added inline comments.

web/assets.js
10–13 ↗	(On Diff #12682)	Maybe we should create a single object with these properties

This revision is now accepted and ready to land.May 17 2022, 5:56 AM

atul added inline comments.May 17 2022, 5:59 AM

web/assets.js
10–13 ↗	(On Diff #12682)	Yeah that would make way more sense. It would let us avoid exporting/importing three separate variables with crazy long names and also clean up the components where they're consumed. I can probably just update this diff in place?