Page MenuHomePhabricator
Differential D7308

[lib/web] Add pin/unpin icon in the tooltip
ClosedPublic
Actions

Authored by rohan on Apr 4 2023, 6:54 PM.
Tags
None
Referenced Files
F2107227: D7308.id25533.diff
Tue, Jun 25, 9:46 AM
Unknown Object (File)
Mon, Jun 24, 11:43 PM
Unknown Object (File)
Mon, Jun 24, 9:02 AM
Unknown Object (File)
Mon, Jun 24, 8:43 AM
Unknown Object (File)
Fri, Jun 21, 8:36 PM
Unknown Object (File)
Tue, Jun 18, 8:33 PM
Unknown Object (File)
Fri, Jun 14, 7:29 AM
Unknown Object (File)
Fri, Jun 14, 2:51 AM
View All Files
Subscribers
ashoat

Details

Reviewers
atul
ginsu
tomek
Commits
rCOMM1b1325634575: [lib/web] Add pin/unpin icon in the tooltip
Summary

We need to display an option in the tooltip to allow users to pin / unpin messages on web.

Linear: https://linear.app/comm/issue/ENG-3445/add-a-pinunpin-icon-in-the-tooltip

These are not the final designs for the pin icons in the tooltip, I've made a task to track adding those icons into the codebase: https://linear.app/comm/issue/ENG-3589/create-the-pin-icons-in-the-codebase

Test Plan

Hover over a pinned message and check to see the unpin icon, and vice versa for unpinned messages.

Screenshot 2023-04-04 at 9.56.14 PM.png (614×1 px, 197 KB)

Screenshot 2023-04-04 at 9.56.26 PM.png (188×868 px, 33 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rohan created this revision.Apr 4 2023, 6:54 PM
Herald added a subscriber: ashoat. · View Herald TranscriptApr 4 2023, 6:54 PM
rohan edited the test plan for this revision. (Show Details)Apr 4 2023, 6:56 PM
rohan attached a referenced file: F463370: Screenshot 2023-04-04 at 9.56.26 PM.png. (Show Details)
rohan attached a referenced file: F463371: Screenshot 2023-04-04 at 9.56.14 PM.png. (Show Details)
rohan added inline comments.
web/modals/chat/toggle-pin-modal.react.js
13 ↗(On Diff #24648)

This is removed in the next diff when I build out this modal

Harbormaster completed remote builds in B17992: Diff 24648.Apr 4 2023, 7:12 PM
rohan requested review of this revision.Apr 4 2023, 7:12 PM
rohan added a child revision: D7309: [web] Pop up a pin/unpin modal when the tooltip icon is clicked.Apr 4 2023, 7:20 PM
tomek accepted this revision.Apr 5 2023, 2:08 AM

Seems like empty toggle-pin-modal.css file was added by accident.

web/modals/chat/toggle-pin-modal.react.js
15 ↗(On Diff #24648)

Doesn't matter too much, but you could also return null here instead.

This revision is now accepted and ready to land.Apr 5 2023, 2:08 AM
rohan added a comment.Apr 5 2023, 5:27 AM
In D7308#217461, @tomek wrote:

Seems like empty toggle-pin-modal.css file was added by accident.

I just made that alongside the same time I made the toggle-pin-modal.react.js file, it's used in D7309!

rohan updated this revision to Diff 25246.Apr 17 2023, 5:45 PM

Use Comm icons for tooltip

Harbormaster completed remote builds in B18431: Diff 25246.Apr 17 2023, 6:04 PM
rohan updated this revision to Diff 25521.Apr 21 2023, 4:40 AM

Refactor parts of tooltip-utils.js out into tooltip-action-utils.js to resolve circular dependencies (should fix the failing unit test)

rohan added inline comments.Apr 21 2023, 4:42 AM
web/utils/tooltip-action-utils.js
1 ↗(On Diff #25521)

The changes to tooltip-action-utils.js and tooltip-utils.js are just a straight file refactoring (no new changes were made)

Harbormaster completed remote builds in B18631: Diff 25521.Apr 21 2023, 4:57 AM
rohan updated this revision to Diff 25533.Apr 21 2023, 7:40 AM

Rebase

Harbormaster completed remote builds in B18642: Diff 25533.Apr 21 2023, 7:58 AM
rohan updated this revision to Diff 25728.Apr 25 2023, 5:31 PM

Rebase

Harbormaster completed remote builds in B18779: Diff 25728.Apr 25 2023, 5:49 PM
Closed by commit rCOMM1b1325634575: [lib/web] Add pin/unpin icon in the tooltip (authored by RohanK6 <rohan.khanderia@gmail.com>, committed by rohan). · Explain WhyApr 25 2023, 6:00 PM
This revision was automatically updated to reflect the committed changes.
rohan added a commit: rCOMM1b1325634575: [lib/web] Add pin/unpin icon in the tooltip.
rohan mentioned this in D8829: [web] introduce useTooltip hook.Aug 22 2023, 1:15 PM

Revision Contents
Changeset List

Diff 25735

View Options

web/CommIcon.react.js

Loading...
View Options

web/chat/composed-message.react.js

Loading...
View Options

web/chat/robotext-message.react.js

Loading...
View Options

web/modals/chat/toggle-pin-modal.css

Loading...
View Options

web/modals/chat/toggle-pin-modal.react.js

Loading...
View Options

web/utils/tooltip-action-utils.js

Loading...
View Options

web/utils/tooltip-utils.js

Loading...