Details

Reviewers

michal
• kuba

Commits

rCOMMe1d183a1ff18: [web] Style the community drawer

Summary

issue: https://linear.app/comm/issue/ENG-2671/style-community-drawer-on-web
Styling the community drawer (only the drawer for now, the top bar and other elements changed during the drawer tasks will be styled in a separate diff).
I made the header 4px taller for now, because until there is a navigation state info in the main content's header (see https://linear.app/comm/issue/ENG-2740/add-navigation-state-info-to-the-top-bar,
https://www.figma.com/file/a1nkbWgbgjRlrOY9LVurTz/Comm-%2F-Desktop-app?node-id=7780%3A136895&t=xSs7qrxIaOTjRJxA-4) the drawers header and main contents header bottom borders are almost aligned, and having them be off by 4px looked bad.
I also talked to the design team and we agreed that the purple line next to the active "Inbox"/"All communities"/"Settings" should be 3px, because otherwise it "sinks under" the browsers border (is not well visible).

Designs

Test Plan

Run web app, check that the styles display correctly.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

inka created this revision.Feb 10 2023, 4:15 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptFeb 10 2023, 4:15 AM

inka edited the summary of this revision. (Show Details)Feb 10 2023, 4:17 AM

inka added a reviewer: • kuba.

inka attached a referenced file: F372984: Screen Recording 2023-02-10 at 12.52.52.mov. (Show Details)

Harbormaster completed remote builds in B16325: Diff 22327.Feb 10 2023, 4:29 AM

inka requested review of this revision.Feb 10 2023, 4:29 AM

Currently, the scroll applies to the whole community picker and not just to the community drawer:

So settings aren't immediately visible and you have to scroll to them. Was it decided somewhere that it should work like this?

web/sidebar/community-picker.react.js
31–34 ↗	(On Diff #22327)	Nit: can we move the always true case to the top? Also, could this be moved closer to the `inboxButtonContainerClass`? (`sideLineX` are near each other so I think that makes sense)

Address code review

@michal you're right! It is supposed to scroll differently. Here is how it works now:

inka attached a referenced file: F376277: Screen Recording 2023-02-13 at 10.31.33.mov. (Show Details)Feb 13 2023, 1:33 AM

Harbormaster completed remote builds in B16369: Diff 22397.Feb 13 2023, 1:46 AM

Nice!

This revision is now accepted and ready to land.Feb 13 2023, 2:10 AM

inka added a parent revision: D6662: [web] Add All communities / Inbox button.Feb 13 2023, 2:20 AM

Rebase

Harbormaster failed remote builds in B16426: Diff 22463!Feb 13 2023, 7:41 AM

Rebase

I talked to the design team, and "Settings" / "Inbox" / "All communities" buttons are not supposed to be highlighted after all. There will be the purple line next to them, but no highlight.

inka requested review of this revision.Feb 14 2023, 3:23 AM

Harbormaster completed remote builds in B16504: Diff 22551.Feb 14 2023, 4:14 AM

Harbormaster completed remote builds in B16508: Diff 22555.Feb 14 2023, 4:28 AM

Remove unused style

michal accepted this revision.Feb 14 2023, 4:36 AM

This revision is now accepted and ready to land.Feb 14 2023, 4:36 AM

Harbormaster completed remote builds in B16511: Diff 22558.Feb 14 2023, 4:50 AM

Closed by commit rCOMMe1d183a1ff18: [web] Style the community drawer (authored by • InkaSokolowska). · Explain WhyFeb 14 2023, 5:02 AM

This revision was automatically updated to reflect the committed changes.

• InkaSokolowska added a commit: rCOMMe1d183a1ff18: [web] Style the community drawer.