Details

Reviewers

tomek
atul
• benschac

Commits

rCOMM926ed5328b17: [web] Introduce `ThreadMember` component

Summary

Introducing component rendering single thread member on the list in thread modal.
It contains menu with actions (added in the next diffs) and displays user role as a label. When the menu is open, the style is a bit different, so it keeps the menu state and receives onMenuChange prop, as the parent component also needs to know the menu status (to disable scrolling when it's open)

Screenshot_Google Chrome_2022-03-09_144343.png (146×347 px, 15 KB)

Test Plan

Component can be tested after introducing thread members modal.

Diff Detail

Repository

rCOMM Comm

Branch

jacek/member-1

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• jacek created this revision.Mar 9 2022, 4:42 AM

Herald added subscribers: • benschac, • karol, atul and 3 others. · View Herald TranscriptMar 9 2022, 4:42 AM

Harbormaster completed remote builds in B7241: Diff 10200.Mar 9 2022, 4:47 AM

• jacek requested review of this revision.Mar 9 2022, 4:47 AM

• jacek edited the summary of this revision. (Show Details)Mar 9 2022, 5:44 AM

• jacek added reviewers: tomek, • benschac, atul.

• jacek added a child revision: D3380: [web] Introduce member action menu items in `ThreadMember` component.Mar 9 2022, 5:53 AM

• jacek added parent revisions: D3378: [web] Introduce `Label` component, D3377: [web] use context to display `Menu` action items, D3376: [web] Introduce additional `Menu` styling variants.

• benschac requested changes to this revision.Mar 10 2022, 8:39 AM

• benschac added inline comments.

web/modals/threads/members/member.react.js
36	why wrap this in `useMemo`? couldnt we just pass the icon name string to the component rather than passing the entire icon component?

This revision now requires changes to proceed.Mar 10 2022, 8:39 AM

• jacek added inline comments.Mar 15 2022, 4:31 AM

web/modals/threads/members/member.react.js
36	The question is related more to `Menu` component design. The idea is, that we can pass different components (in an `icon` prop), that open menu after click - and they may be differently styled.

Make sure to re-request review if you're responding to a comment without any updates! Otherwise, the diff stays in your queue

Rebase & moved onMenuChange callback into the component

Harbormaster completed remote builds in B7415: Diff 10439.Mar 16 2022, 12:00 PM

• benschac resigned from this revision.Mar 16 2022, 12:32 PM

This comment was removed by • benschac.

tomek accepted this revision.Mar 17 2022, 9:18 AM

tomek added inline comments.

web/modals/threads/members/member.react.js
21 ↗	(On Diff #10439)	Maybe `SetState<?string>` can be used instead?
34–40 ↗	(On Diff #10439)	This can be simplified significantly
46 ↗	(On Diff #10439)	Why do we declare this, never change and then render? Is it because it is updated in next diffs?