[web][landing] Fix miscellaneous CSS issues
ClosedPublic
Actions

Authored by atul on Jun 22 2022, 7:13 PM.

Details

Reviewers

tomek
• jacek
• abosh

Commits

rCOMM4bea17c16501: [web][landing] Fix miscellaneous CSS issues

Summary

Ran the code analysis tool in IDE which surfaced some misc CSS issues. I'll label each issue inline.

Depends on D4338

Test Plan

NA, careful reading

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.Jun 22 2022, 7:13 PM

Herald added subscribers: • adrian, ashoat. · View Herald TranscriptJun 22 2022, 7:13 PM

fix

atul added inline comments.Jun 22 2022, 7:16 PM

landing/info-block.css
31 ↗	(On Diff #13695)	We should include a fallback font here. In this case `sans-serif`
31 ↗	(On Diff #13694)	W
31 ↗	(On Diff #13694)	We should include a fallback here
landing/subscription-form.css
10 ↗	(On Diff #13695)	Properties could be collapsed into single line
web/components/menu.css
41 ↗	(On Diff #13695)	`background-color` and `color` were included twice (with the same values each time)
web/theme.css
129 ↗	(On Diff #13695)	Typo, `--shades-of-white-60` doesn't exist

Harbormaster completed remote builds in B9902: Diff 13694.Jun 22 2022, 7:18 PM

Harbormaster completed remote builds in B9903: Diff 13695.Jun 22 2022, 7:23 PM

atul requested review of this revision.Jun 22 2022, 7:23 PM

atul added a child revision: D4340: [lib] Merge identical action handlers in `message-reducer`.Jun 22 2022, 8:33 PM

atul edited the summary of this revision. (Show Details)Jun 22 2022, 8:33 PM

atul added a parent revision: D4338: [lib] Return `mergeNewMessages(...)` directly where possible in `reduceMessageStore(...)`.

tomek accepted this revision.Jun 23 2022, 4:56 AM

tomek added inline comments.

landing/info-block.css
31 ↗	(On Diff #13695)	Can we define something like `--font-stack` on web?
web/theme.css
129 ↗	(On Diff #13695)	Please check if this change modifies how the result looks like

This revision is now accepted and ready to land.Jun 23 2022, 4:56 AM

• abosh accepted this revision.Jun 23 2022, 8:16 AM

• abosh added inline comments.

landing/subscription-form.css
10 ↗	(On Diff #13695)	Don't need units for 0. Read more: https://stackoverflow.com/q/7923042/13079675
65 ↗	(On Diff #13695)

• abosh added a child revision: D4343: [web] [landing] Omit units for 0 values in CSS.Jun 23 2022, 8:39 AM

atul added inline comments.Jun 24 2022, 11:18 AM

landing/info-block.css
31 ↗	(On Diff #13695)	Created a task: https://linear.app/comm/issue/ENG-1293/define-something-like-font-stack-on-web and added to backlog.
landing/subscription-form.css
10 ↗	(On Diff #13695)	Saw that this was handled on another diff
web/theme.css
129 ↗	(On Diff #13695)	It looks like it fixes an issue. Before: After: Before this fix, the active thread text was 100% white (255,255,255) which made it look unread. This fixes the styling for active && read threads.

rebase after cherrypicking and before landing

atul attached a referenced file: F82840: 44cbde.png. (Show Details)Jun 24 2022, 11:20 AM

atul attached a referenced file: F82841: 764414.png. (Show Details)

FYI, I can't see the images in this inline comment. I ran into a similar issue when reviewing another diff, so maybe this is an issue where images don't show up on inline comments.