Page MenuHomePhabricator

[web] Add background illustration to background tab
ClosedPublic

Authored by atul on May 2 2022, 11:55 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 5, 4:35 AM
Unknown Object (File)
Sat, Oct 19, 11:58 AM
Unknown Object (File)
Sat, Oct 19, 11:58 AM
Unknown Object (File)
Sat, Oct 19, 11:58 AM
Unknown Object (File)
Sat, Oct 19, 11:58 AM
Unknown Object (File)
Sat, Oct 19, 11:58 AM
Unknown Object (File)
Sat, Oct 19, 11:58 AM
Unknown Object (File)
Sat, Oct 19, 11:54 AM

Details

Summary

We already did this on mobile a while ago.

As part of https://phabricator.ashoat.com/D3856 ("[native] Bump react-native-svg to 12.3.0") I double checked that the background illustration continued to look as expected on native... at which point I realized we never made the corresponding change on web... so decided to do it really quick.

Test Plan

Here's what it looks like:

ca29.png (760×826 px, 75 KB)

Diff Detail

Repository
rCOMM Comm
Branch
landmay2 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul published this revision for review.May 2 2022, 11:55 AM
atul edited the test plan for this revision. (Show Details)
atul edited the summary of this revision. (Show Details)
atul edited the summary of this revision. (Show Details)

(I definitely think the EmptyItem component could be styled better overall... however, this diff is really just about adding the illustration)

Assuming there are no build failures

This revision is now accepted and ready to land.May 2 2022, 11:59 AM
atul edited the test plan for this revision. (Show Details)

include asset height and width: noticed that there was a stutter/reflow without this change

before:

after:

web/assets.js
7–8 ↗(On Diff #12147)

It doesn't seem great to be hardcoding values here like this, open to any alternative suggestions/approaches. Didn't want to spend too much time on this task, but I think this is fine for now?

web/assets.js
7–8 ↗(On Diff #12147)

Yeah that's annoying... I guess if we were smart, then we would have the SVG in the repo, have Terraform handle automatically persisting it to Cloudfront whenever it changes, and have the values here generated from the SVG either statically (at commit time) or during SSR (in Node) or something

atul marked an inline comment as done.May 2 2022, 1:06 PM
atul added inline comments.
web/assets.js
7–8 ↗(On Diff #12147)

Ah yeah that makes sense.. so something to defer for later

atul marked an inline comment as done.

rebase before landing

This revision was landed with ongoing or failed builds.May 4 2022, 6:22 AM
This revision was automatically updated to reflect the committed changes.