Page MenuHomePhabricator

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

Authored by rohan on Apr 4 2023, 6:54 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 28, 4:16 PM
Unknown Object (File)
Wed, Dec 25, 11:24 AM
Unknown Object (File)
Mon, Dec 9, 1:51 PM
Unknown Object (File)
Sat, Dec 7, 8:33 PM
Unknown Object (File)
Sat, Dec 7, 8:33 PM
Unknown Object (File)
Sat, Dec 7, 8:33 PM
Unknown Object (File)
Sat, Dec 7, 7:25 AM
Unknown Object (File)
Sat, Dec 7, 7:25 AM
Subscribers

Details

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 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

rohan requested review of this revision.Apr 4 2023, 7:12 PM

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
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!

Use Comm icons for tooltip

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

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)