[web] [darkmode] unread button
ClosedPublic
Actions

Authored by • benschac on Feb 11 2022, 11:04 AM.

Details

Reviewers

atul
ashoat

Commits

rCOMMfb93ae4b58ba: [web] [darkmode] unread button

Summary

theme unread button, no design in web. Generally just theme'ed with our color palette.

Image 2022-02-11 at 2.00.25 PM.jpg (312×440 px, 17 KB)

Adding ashoat since Atul is OOO today.

Test Plan

should work as before.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• benschac created this revision.Feb 11 2022, 11:04 AM

Herald added subscribers: • karol, • adrian, tomek. · View Herald TranscriptFeb 11 2022, 11:04 AM

• benschac edited the summary of this revision. (Show Details)Feb 11 2022, 11:06 AM

• benschac edited the test plan for this revision. (Show Details)

Harbormaster completed remote builds in B6755: Diff 9576.Feb 11 2022, 11:08 AM

• benschac requested review of this revision.Feb 11 2022, 11:08 AM

Is @atul still OOO this week? Why was I added here but not to D3180?

web/chat/chat-thread-list-item-menu.react.js
78 ↗	(On Diff #9576)	Since you're not using `menuContentVisible` anymore, you should delete the associated CSS selector in `chat-thread-list.css`

This revision now requires changes to proceed.Feb 13 2022, 8:57 PM

remove css class

In D3181#84766, @ashoat wrote:

Is @atul still OOO this week? Why was I added here but not to D3180?

Not that I know of.
Thought there was more urgency here because unread is a important visual indicator.

Harbormaster completed remote builds in B6783: Diff 9605.Feb 14 2022, 4:56 AM

Looks good. Outside the scope of this diff, but:

I think we should find and use a vertical ellipsis icon without a small hole in each circle.
We should give the Mark as read button a drop shadow so there's some depth.

web/chat/chat-thread-list-item-menu.react.js
78 ↗	(On Diff #9605)	Can just do `let btn;`
92 ↗	(On Diff #9605)	Does this need to be in a `<div>`? Could we just do `{btn}`?
web/components/button.react.js
11 ↗	(On Diff #9605)	Instead of `unread`, which might not be clear to the reader.. maybe `toggle_read` or `toggle_read_status` or something?

@benschac, can you make sure to create follow-ups for the tasks @atul identified before landing?

This revision is now accepted and ready to land.Feb 14 2022, 9:33 PM

In D3181#85170, @atul wrote:

Looks good. Outside the scope of this diff, but:

I think we should find and use a vertical ellipsis icon without a small hole in each circle.

We should give the Mark as read button a drop shadow so there's some depth.

That's quite literally what's in Figma. We'd need to find a different icon pack or get someone to make an icon. https://linear.app/comm/issue/ENG-770/fix-vertical-ellipsis-icon-so-it-doesnt-have-a-hole-in-the-middle-of
https://linear.app/comm/issue/ENG-769/mark-as-read-button-a-drop-shadow-so-theres-some-depth

address diff comments

web/chat/chat-thread-list-item-menu.react.js
92 ↗	(On Diff #9605)	yes. I didn't want to refactor the code even more.

This revision was landed with ongoing or failed builds.Feb 15 2022, 6:46 AM

Closed by commit rCOMMfb93ae4b58ba: [web] [darkmode] unread button (authored by • benschac). · Explain Why

This revision was automatically updated to reflect the committed changes.

• benschac added a commit: rCOMMfb93ae4b58ba: [web] [darkmode] unread button.

Harbormaster completed remote builds in B6833: Diff 9673.Feb 15 2022, 6:50 AM

In D3181#85170, @atul wrote:

Looks good. Outside the scope of this diff, but:

I think we should find and use a vertical ellipsis icon without a small hole in each circle.

We should give the Mark as read button a drop shadow so there's some depth.