Page MenuHomePhabricator
Differential D3545

[web] Increase clickable region of `ChatThreadListSidebar`
ClosedPublic
Actions

Authored by atul on Mar 29 2022, 12:34 PM.
Tags
None
Referenced Files
F2899297: D3545.id10902.diff
Sat, Oct 5, 6:23 AM
F2899287: D3545.id10901.diff
Sat, Oct 5, 6:22 AM
F2898994: D3545.diff
Sat, Oct 5, 4:46 AM
Unknown Object (File)
Thu, Sep 19, 5:17 PM
Unknown Object (File)
Thu, Sep 19, 5:17 PM
Unknown Object (File)
Thu, Sep 19, 5:17 PM
Unknown Object (File)
Thu, Sep 19, 5:17 PM
Unknown Object (File)
Thu, Sep 19, 5:12 PM
View All Files
Subscribers
• abosh
• adrian
• benschac
• karol

Details

Reviewers
ashoat
tomek
• jacek
Commits
rCOMM852cfc762459: [web] Increase clickable region of `ChatThreadListSidebar`
Summary

Basically D3543 but for ChatThreadListSidebar

Clickable area before:

d08a.png (228×800 px, 23 KB)

Clickable area after:

000a.png (66×822 px, 8 KB)

Test Plan

Looks and behaves as expected: [will drag in video on phabricator]

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul created this revision.Mar 29 2022, 12:34 PM
Herald added subscribers: • abosh, • benschac, • karol, • adrian. · View Herald TranscriptMar 29 2022, 12:34 PM
atul edited the summary of this revision. (Show Details)Mar 29 2022, 12:39 PM
atul edited the test plan for this revision. (Show Details)
atul added inline comments.
web/chat/chat-thread-list-sidebar.react.js
25

"Why not remove the useOnClickThread(threadInfo) from SidebarItem if we moved it out here?"

SidebarItem is also used in the Sidebar modal and so I didn't want to break that (any further) with this change. Sidebar modal requires a lot of work, so I'll make sure to address the redundancy then.

Harbormaster completed remote builds in B7686: Diff 10788.Mar 29 2022, 12:45 PM
atul requested review of this revision.Mar 29 2022, 12:45 PM
tomek accepted this revision.Mar 30 2022, 3:17 AM

Like in D3543 I'm not sure about extending the area near ellipsis button

web/chat/chat-thread-list-sidebar.react.js
25

But aren't we going to fire an event twice if we keep both handlers?

This revision is now accepted and ready to land.Mar 30 2022, 3:17 AM
atul added inline comments.Mar 30 2022, 9:36 AM
web/chat/chat-thread-list-sidebar.react.js
25

I believe event.stopPropogation() should prevent that

atul added a comment.Mar 30 2022, 9:44 AM
In D3545#97054, @palys-swm wrote:

Like in D3543 I'm not sure about extending the area near ellipsis button

addressed in D3543: https://phabricator.ashoat.com/D3543#97252

ashoat accepted this revision.Mar 30 2022, 11:03 PM
atul updated this revision to Diff 10901.Mar 31 2022, 9:18 AM

rebase before landing

This revision was landed with ongoing or failed builds.Mar 31 2022, 9:22 AM
Closed by commit rCOMM852cfc762459: [web] Increase clickable region of `ChatThreadListSidebar` (authored by atul). · Explain Why
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMM852cfc762459: [web] Increase clickable region of `ChatThreadListSidebar`.
Harbormaster completed remote builds in B7772: Diff 10901.Mar 31 2022, 9:28 AM

Revision Contents
Changeset List

Diff 10788

View Options

web/chat/chat-thread-list-sidebar.react.js

Loading...
View Options

web/chat/chat-thread-list.css

Loading...