rendered thread avatars for both parent and child threads in thread settings
Details
Details
Please check out these screenshots to see the changes I made:
staff user:
non staff user:
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| native/chat/settings/thread-settings-child-thread.react.js | ||
|---|---|---|
| 34 ↗ | (On Diff #23968) | Why do we need to use useResolvedThreadInfo? |
| native/chat/settings/thread-settings-parent.react.js | ||
|---|---|---|
| 19 | I created this component here so that I could pass in the parentThreadInfo as ThreadInfo instead of ?ThreadInfo into the useGetAvatarForThread hook, I figured since this is such a small component and is used just here for now, we could keep the component in this file; however, if we should keep this in a separate file, lmk and I will quickly do that | |
Comment Actions
Accepting to unblock but the pattern of copy-paste is concerning
| native/chat/settings/thread-settings-child-thread.react.js | ||
|---|---|---|
| 33–45 | This is basically copy-pasted from D7136, with a different size. Why don't we extract this into a component? | |
| native/chat/settings/thread-settings-parent.react.js | ||
| 31–41 | Seems like more copy-paste. As I've mentioned before: when you copy-paste code, an alarm should be ringing through your head. You shouldn't wait until you get comments from a reviewer to address it | |
Comment Actions
Accepting to unblock but the pattern of copy-paste is concerning
| native/chat/settings/thread-settings-child-thread.react.js | ||
|---|---|---|
| 33–45 | Created a task to track this concern and should offer more context on why we didn't extract from the beginning. https://linear.app/comm/issue/ENG-3413/factor-out-copy-pasted-feature-flag-logic-code | |
| native/chat/settings/thread-settings-parent.react.js | ||
| 31–41 | ||