Page MenuHomePhabricator

[web] Match `ChatInputBar` icon colors with thread color
ClosedPublic

Authored by abosh on May 4 2022, 10:58 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 3, 2:20 PM
Unknown Object (File)
Sun, Nov 3, 2:20 PM
Unknown Object (File)
Sun, Nov 3, 2:20 PM
Unknown Object (File)
Sun, Nov 3, 2:16 PM
Unknown Object (File)
Fri, Nov 1, 5:26 AM
Unknown Object (File)
Oct 10 2024, 4:17 AM
Unknown Object (File)
Oct 2 2024, 2:18 PM
Unknown Object (File)
Sep 25 2024, 3:05 AM

Details

Summary

Changed multimedia upload and send button icon colors on ChatInputBar on web to match the thread color, similar to how it looks
on native:

image.png (1×716 px, 727 KB)

Related Linear issue with full context.

Test Plan

Tested on Chrome/Safari and works as expected:
Before:

image.png (450×1 px, 65 KB)

After:
image.png (244×1 px, 33 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

abosh edited the test plan for this revision. (Show Details)
abosh added 1 blocking reviewer(s): atul.
web/chat/chat-input-bar.react.js
232 ↗(On Diff #12230)

The reason this is an inline style instead of using the color prop in SWMansionIcon is because the color prop did not result in any changes -- so the style had to be overwritten using the style prop. Not sure why the color prop isn't working, but considering the color prop works fine on native for SWMansionIcon, this should be fixed on web.

255 ↗(On Diff #12230)

If this were a functional component, this inline style would be memoized using useMemo so it would only re-render if the thread color actually changed. However, this is a class component, so I'm not sure the best way forward. We could pass the thread color in ConnectedChatInputBar at the bottom of this file. This file also contains other non-memoized inline styles, so this should be worked out.

atul added inline comments.
web/chat/chat-input-bar.react.js
232 ↗(On Diff #12230)

Styling SWMansionIcons on web is a known issue. I think this approach is fine for now as it matches what's being done elsewhere in this file.

255 ↗(On Diff #12230)

We could pass the thread color in ConnectedChatInputBar at the bottom of this file.

Just a note, memoizing the thread color--which is of type string--and passing it down from ConnectedChatInputBar wouldn't help us here. What we'd want to memoize instead is the style object (eg { color: '#FFFFFF' } or whatever). The benefit of memoizing objects is to maintain referential equality. There's no benefit to memoizing strings

This revision is now accepted and ready to land.May 5 2022, 5:25 AM
abosh added inline comments.
web/chat/chat-input-bar.react.js
255 ↗(On Diff #12230)

Ah, you're right.

This revision was automatically updated to reflect the committed changes.
abosh marked an inline comment as done.