Page MenuHomePhabricator

[web] Use new style prop passed from getMessagePreview
ClosedPublic

Authored by ashoat on Jan 23 2023, 2:53 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 12, 12:26 AM
Unknown Object (File)
Fri, Nov 8, 2:59 AM
Unknown Object (File)
Fri, Nov 8, 2:59 AM
Unknown Object (File)
Fri, Nov 8, 2:58 AM
Unknown Object (File)
Fri, Nov 8, 2:58 AM
Unknown Object (File)
Tue, Oct 22, 3:18 AM
Unknown Object (File)
Tue, Oct 22, 3:18 AM
Unknown Object (File)
Tue, Oct 22, 3:18 AM
Subscribers
None

Details

Summary

This diff results in only one visual difference: when the most recent message in a read thread is a robotext message, it is displayed with the darker "secondary" color. This brings web behavior to match native, but I'm also open to the alternative (making native behave like web).

Will annotate CSS changes inline, as they're somewhat complicated.

Depends on D6357

Test Plan
beforeafter
Screenshot 2023-01-23 at 5.25.45 PM.png (1×814 px, 136 KB)
Screenshot 2023-01-23 at 5.24.25 PM.png (1×812 px, 135 KB)

(identical except "ashoat added commbot" near the bottom)

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Rename read CSS selector to messagePreviewPrimary

web/chat/chat-thread-list.css
27

This selector should match the same as before, but I think it's more readable this way. We're saying "the title, the message preview, and anything inside the message preview". The third part is necessary to override the color set by the MessagePreview JS on usernames specifically – otherwise this selector only applies the color to the username span's parent, which has lower precedence than a less-specific selector applied directly to the child

Previously div.dark was taking the place of .lastMessage, and .lastMessage span.read was taking the place of .lastMessage *. I believe div.dark appears in the same places under .activeThread that .lastMessage does, and I don't think we need to limit this color change to read threads (as an active thread cannot be unread)

31

This is handled by the updated selector above

126

I think before @inka pulled out the sidebar modal into its own CSS file, this selector needed to be separate from the above. But now that we're only using these selectors for MessagePreview, we can unify them

127

This is basically a "default color". We take out this default color here to avoid worrying about CSS precedence issues. It's easier if we only apply a single color (except in the case of the thread being active, where we apply a second color with higher CSS precedence – see above)

For details on how this was replaced, see comment under messagePreviewPrimary below

130

This selector takes the place of the "default color" in div.threadRow > .lastMessage. Replacing it allows us to deprecate --thread-last-message-color-read (deduped with --thread-color-read)

Note that this "default" is now only used for the text of a text message preview on a read thread. (Previously it was also used for robotext message previews)

134–136

This is covered by the above selector.

When we were applying multiple colors, we needed the unread color to have higher precedence, so we defined this more-highly-specified CSS selector to override everything else. Now that we are avoiding applying multiple colors, we don't need this selector

141–143

I confirmed that there are no users of this selector outside of MessagePreview before removing it.

Previously, we were only using this selector for the color of the username prefix for read threads. For the rest of the message, it was being overriden due to CSS precedence by the more-highly-specified div.threadRow > .lastMessage selector

At the time, it was being using basically as the "secondary" color. It's replaced by messagePreviewSecondary

131 ↗(On Diff #21235)

This change can be a little hard to grok, as we're actually changing the color (and the name) of this selector.

Previously, we were only using this selector for the color of the username prefix for read threads. For the rest of the message, it was being overriden due to CSS precedence by the more-highly-specified div.threadRow > .lastMessage selector. At the time, it was being using basically as the "secondary" color.

Now, we are only using this selector for the text of a text message preview on a read thread. That corresponds to the "primary" color. I confirmed that the one usage in MessagePreview is the only place in the codebase that uses this selector.

web/chat/chat-thread-list.css
131 ↗(On Diff #21235)

Ignore this comment, I forgot to delete it

This revision is now accepted and ready to land.Jan 24 2023, 6:30 AM