Page MenuHomePhabricator
Differential D3889

[web] Add background illustration to background tab
ClosedPublic
Actions

Authored by atul on May 2 2022, 11:55 AM.
Tags
None
Referenced Files
F3359384: D3889.diff
Sun, Nov 24, 8:49 AM
Unknown Object (File)
Fri, Nov 15, 6:10 AM
Unknown Object (File)
Thu, Nov 14, 9:07 AM
Unknown Object (File)
Thu, Nov 14, 9:07 AM
Unknown Object (File)
Thu, Nov 14, 9:07 AM
Unknown Object (File)
Mon, Nov 11, 6:42 PM
Unknown Object (File)
Mon, Nov 11, 5:44 PM
Unknown Object (File)
Mon, Nov 11, 3:48 PM
View All Files
Subscribers
• abosh
• adrian
• karol

Details

Reviewers
• jacek
tomek
• benschac
ashoat
Commits
rCOMMc7fdc0b41542: [web] Add background illustration to background tab
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 created this revision.May 2 2022, 11:55 AM
Herald added subscribers: • abosh, • karol, • adrian. · View Herald TranscriptMay 2 2022, 11:55 AM
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)
atul added a comment.EditedMay 2 2022, 11:58 AM

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

ashoat accepted this revision.May 2 2022, 11:59 AM

Assuming there are no build failures

This revision is now accepted and ready to land.May 2 2022, 11:59 AM
Harbormaster completed remote builds in B8720: Diff 12146.May 2 2022, 12:02 PM
atul updated this revision to Diff 12147.EditedMay 2 2022, 12:06 PM
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:

atul added inline comments.May 2 2022, 12:07 PM
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?

Harbormaster completed remote builds in B8721: Diff 12147.May 2 2022, 12:15 PM
ashoat added inline comments.May 2 2022, 12:27 PM
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 updated this revision to Diff 12158.May 2 2022, 1:07 PM
atul marked an inline comment as done.

rebase before landing

Harbormaster completed remote builds in B8727: Diff 12158.May 2 2022, 1:12 PM
atul updated this revision to Diff 12211.May 4 2022, 6:20 AM

rebase before landign

This revision was landed with ongoing or failed builds.May 4 2022, 6:22 AM
Closed by commit rCOMMc7fdc0b41542: [web] Add background illustration to background tab (authored by atul). · Explain Why
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMMc7fdc0b41542: [web] Add background illustration to background tab.
Harbormaster completed remote builds in B8765: Diff 12211.May 4 2022, 7:08 AM

Revision Contents
Changeset List

PathSize
web/
8 lines
chat/
10 lines
17 lines

Diff 12158

View Options

web/assets.js

Loading...
View Options

web/chat/chat-thread-list.css

Loading...
View Options

web/chat/chat-thread-list.react.js

Loading...