Page MenuHomePhabricator

[web] factor out pinned messages banner into its own component
ClosedPublic

Authored by ginsu on Jan 5 2024, 12:09 AM.
Tags
None
Referenced Files
F3376531: D10548.id35260.diff
Wed, Nov 27, 12:49 AM
F3376460: D10548.id35261.diff
Wed, Nov 27, 12:25 AM
Unknown Object (File)
Sat, Nov 9, 4:26 PM
Unknown Object (File)
Fri, Nov 8, 4:48 PM
Unknown Object (File)
Sun, Nov 3, 3:15 AM
Unknown Object (File)
Sun, Nov 3, 3:15 AM
Unknown Object (File)
Sun, Nov 3, 3:15 AM
Unknown Object (File)
Sun, Nov 3, 3:15 AM
Subscribers

Details

Summary

We will be using the thread top bar component as the panel header for the message list panel item. In order to accomplish this we will need to decouple the pinned messages banner from the top bar. To make the move a bit easier/more organized, I took a second to make the pinned messages banner into its own component.

Additionally, I snuck in some color changes that have the pinned messages banner utilize the new color design system.

Test Plan

flow and confirmed the pinned messages banner still works as expected. Also please see the screenshots below:

dark:

Screenshot 2024-01-11 at 12.17.03 PM.png (210×1 px, 24 KB)

light:

Screenshot 2024-01-11 at 12.19.20 PM.png (116×822 px, 13 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu requested review of this revision.Jan 5 2024, 12:46 AM

You are changing some colours here, can you explain why/what it changes?

Personally think it's fine you snuck the color name change in this diff, but in the future we should definitely make a note of that in the Summary section and annotate the change in the diff.

This revision is now accepted and ready to land.Jan 9 2024, 11:32 PM
ginsu edited the test plan for this revision. (Show Details)

Personally think it's fine you snuck the color name change in this diff, but in the future we should definitely make a note of that in the Summary section and annotate the change in the diff.

My bad for not calling this out earlier. I was trying to "take out two birds with one diff" in regards with also updating the colors in the pinned messages banner to utilize the new color design system. I'll be more careful about this in the future, and will make sure that these types are changes are super clear in the summary + annotated before the first pass of review.

I have updated the test plan to include screenshots of the pinned messages banner in both dark + light mode