[web] Combine constants in `web/assets.js` into objects
ClosedPublic
Actions

Authored by atul on May 23 2022, 1:29 PM.

Details

Reviewers

tomek
• jacek
• benschac

Commits

rCOMM9c42d0846821: [web] Combine constants in `web/assets.js` into objects

Summary

Addresses feedback from @palys-swm in D4041.
Linear Issue: https://linear.app/comm/issue/ENG-1172/combine-constants-in-webassetsjs-into-objects

Test Plan

Loaded the web app
Navigated to background tab and ensured that illustration looked as expected
Navigated to NotificationsModal and ensured that illustrations look as expected

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.May 23 2022, 1:29 PM

Herald added subscribers: • abosh, • adrian, ashoat. · View Herald TranscriptMay 23 2022, 1:29 PM

Harbormaster completed remote builds in B9399: Diff 13019.May 23 2022, 1:35 PM

atul requested review of this revision.May 23 2022, 1:35 PM

Nice! We can also consider creating a component that takes an image asset and renders img with the asset. It might make sense because in every place we use the asset, we render an img and have to type asset name thrice.

This revision is now accepted and ready to land.May 24 2022, 1:02 AM

rebase before landing

We can also consider creating a component that takes an image asset and renders img with the asset. It might make sense because in every place we use the asset, we render an img and have to type asset name thrice.

Yeah that makes sense, will take a look at that when working on the icon/graphics stuff later this week. I think there might be better ways for us to include SVG assets than host them "remotely" (on S3) and pass URI to <img> tag

Harbormaster completed remote builds in B9434: Diff 13068.May 24 2022, 6:26 AM

Closed by commit rCOMM9c42d0846821: [web] Combine constants in `web/assets.js` into objects (authored by atul). · Explain WhyMay 24 2022, 6:27 AM

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMM9c42d0846821: [web] Combine constants in `web/assets.js` into objects.

will take a look at that when working on the icon/graphics stuff later this week

You should always link a Linear task when deferring work like this

In D4107#116475, @ashoat wrote:

will take a look at that when working on the icon/graphics stuff later this week

You should always link a Linear task when deferring work like this

https://linear.app/comm/issue/ENG-1192/figure-out-how-to-display-non-icon-graphicsillustrations-on-webnative