[web] Fix: Inbox spacing regression
ClosedPublic
Actions

Authored by • jacek on Jul 28 2022, 3:49 AM.

Details

Reviewers

tomek
atul
ashoat

Commits

rCOMMa392b9af4a6e: [web] Fix: Inbox spacing regression

Summary

Fix for issue: https://linear.app/comm/issue/ENG-1473/inbox-spacing-regression-on-safari-on-web
~~I used CSS child selector to avoid modifying reusable Tabs component~~
Modified Tabs component CSS by making it fill the container.

Test Plan

Log in as a user that doesn't have any thread (or first message in the thread) with long name. The chat list should now fill the container.
Tested on Chrome, Safari and Firefox.

Diff Detail

Repository

rCOMM Comm

Branch

jacek/fix-regression

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• jacek created this revision.Jul 28 2022, 3:49 AM

Herald added subscribers: • abosh, • adrian. · View Herald TranscriptJul 28 2022, 3:49 AM

Harbormaster completed remote builds in B10933: Diff 15042.Jul 28 2022, 4:01 AM

• jacek requested review of this revision.Jul 28 2022, 4:01 AM

tomek requested changes to this revision.Jul 28 2022, 5:24 AM

tomek added inline comments.

web/chat/chat-tabs.css
8–15	Targeting all the children sounds risky... maybe we can add style to `.tabItem`?

This revision now requires changes to proceed.Jul 28 2022, 5:24 AM

• jacek requested review of this revision.Jul 28 2022, 5:35 AM

• jacek added inline comments.

web/chat/chat-tabs.css
8–15	The `.tabItem` class is not used now. The only child of `div.container` is `Tabs` component at the moment, which has its own styles (and I didn't want to change them, because it would affect all `Tabs` in the app).

• jacek planned changes to this revision.Jul 28 2022, 5:49 AM

Accepting to unblock a fix for this regression, but agree with @tomek's sentiment... wonder if there's way to do this without targetting all children of div.container

web/chat/chat-tabs.css
8–15	What would happen if all tabs in the app have `flex: 1`? Would it be bad?

Updated Tabs component after discussion with Tomek

• jacek edited the summary of this revision. (Show Details)Jul 28 2022, 6:26 AM

• jacek edited the test plan for this revision. (Show Details)

• jacek added inline comments.

web/chat/chat-tabs.css
8–15	After discussing with Tomek we agreed, that having `flex: 1` in all tabs in app is ok, and I didn't notice any problems with existing usages of tabs after the change.