Page MenuHomePhabricator

[web] introduce community header
ClosedPublic

Authored by ginsu on Dec 29 2023, 12:33 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 31, 3:53 AM
Unknown Object (File)
Thu, Oct 31, 3:53 AM
Unknown Object (File)
Thu, Oct 31, 3:53 AM
Unknown Object (File)
Thu, Oct 31, 3:53 AM
Unknown Object (File)
Thu, Oct 31, 3:53 AM
Unknown Object (File)
Thu, Oct 31, 3:52 AM
Unknown Object (File)
Oct 3 2024, 12:01 PM
Unknown Object (File)
Sep 21 2024, 12:50 PM
Subscribers

Details

Summary

This diff puts all the community header components I built in previous diffs and introduces it as the community header component. Just also want to note that this diff also includes some "hacky" logic I introduced to test the two main states of this header. This logic will be removed once I start working on improving the navigation state when navigating communites:

  1. When home/all communities is selected (there should be no community action buttons and the header should say "All communities")

Screenshot 2023-12-29 at 3.36.27 PM.png (1×1 px, 225 KB)

  1. When a community is selected (the name/avatar of the community should be in the header and the community action buttons should be rendered)

Screenshot 2023-12-29 at 3.36.20 PM.png (1×1 px, 320 KB)

Linear task: https://linear.app/comm/issue/ENG-5965/top-bar-when-a-community-is-selected and https://linear.app/comm/issue/ENG-5964/top-bar-when-inboxhome-is-selected

Depends on D10498

Test Plan

Please see the screenshots below

When home/all communities is selected:

Screenshot 2023-12-29 at 3.39.42 PM.png (1×3 px, 809 KB)

When a community is selected:

Screenshot 2023-12-29 at 3.39.37 PM.png (1×3 px, 817 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

web/community-header/community-header.react.js
13–15 ↗(On Diff #35109)
18–20 ↗(On Diff #35109)
ginsu requested review of this revision.Dec 29 2023, 1:00 PM
atul requested changes to this revision.Dec 30 2023, 2:49 AM
atul added a subscriber: varun.

Looks good, but we should try to remove "hard coded" debugging values. Feel free to re-request changes if there's something I'm missing.

web/community-header/community-header.react.js
14–15 ↗(On Diff #35109)

Eh, not sure how I feel about having these hard coded values (and commented out values) checked into the repo... I think this is a pattern we generally try to avoid.

Approaches I've used instead to try to keep git history clean:

  • For simple things, git stashing and git stash popping to remove/bring back these sorts of debugging changes
  • Have one (or more) [DO NOT LAND] commits in my local branch that make these sorts of temporary changes without having to deal with stashing and unstashing. Have to make sure to remove these from stack before landing.

CC @ashoat, @tomek, @varun for thoughts

45–50 ↗(On Diff #35109)

Wouldn't hurt to memoize

This revision now requires changes to proceed.Dec 30 2023, 2:49 AM
web/community-header/community-header.react.js
14–15 ↗(On Diff #35109)

Yeah I agree

ginsu edited the summary of this revision. (Show Details)

address comments

Thanks for addressing feedback

This revision is now accepted and ready to land.Jan 3 2024, 11:04 AM
This revision was landed with ongoing or failed builds.Jan 3 2024, 11:53 AM
This revision was automatically updated to reflect the committed changes.