Page MenuHomePhabricator

[native] added arrows for threads in chat inbox
ClosedPublic

Authored by ginsu on Sep 29 2022, 2:06 PM.
Tags
None
Referenced Files
F3526330: D5268.diff
Mon, Dec 23, 9:46 PM
Unknown Object (File)
Tue, Nov 26, 3:22 PM
Unknown Object (File)
Tue, Nov 26, 3:22 PM
Unknown Object (File)
Tue, Nov 26, 3:21 PM
Unknown Object (File)
Sun, Nov 24, 12:23 PM
Unknown Object (File)
Nov 5 2024, 9:58 AM
Unknown Object (File)
Oct 19 2024, 1:47 AM
Unknown Object (File)
Oct 19 2024, 1:47 AM

Details

Summary

Implemented the arrows design for threads in chat inbox. Removed the icon, and moved the unread dot logic from chat-thread-list-item.react to chat-thread-list-sidebar

Next steps/diffs:

  1. Include arrow design to be a part of SidebarListModal as well

won't land this diff until the next diff is also approved and ready to land

Linear Task: ENG-1779

Depends on D5253 and D5264

Test Plan

Please view the video and the Figma designs attached to review the visual changes I made

Before:

Screen Shot 2022-09-29 at 5.18.51 PM.png (1×1 px, 675 KB)

After:

Figma:
Normal State
Swipe State

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

native/chat/chat-thread-list-sidebar.react.js
30–42 ↗(On Diff #17210)

Used the web Sidebar Item component as inspiration

ginsu requested review of this revision.Sep 29 2022, 2:17 PM
atul requested changes to this revision.EditedSep 30 2022, 11:59 AM

The demo in the diff description looks great! However, I don't think we can move the UnreadDot from SidebarItem to ChatThreadListSidebar so easily..

Based on a quick search of the codebase we use SidebarItem in other components (eg SidebarListModal)... are we sure that this change won't break things there? At a high level it's always good to look through the codebase for other usages of a component to see if there may be any unintended visual side effects.

But definitely let me know if I'm missing something/you already considered other usages of the component/etc

Discussed IRL, all other cases were considered by @ginsu

This revision now requires changes to proceed.Sep 30 2022, 11:59 AM
ginsu edited the summary of this revision. (Show Details)

Discussed IRL

This revision is now accepted and ready to land.Oct 3 2022, 1:40 PM