Page MenuHomePhabricator
Differential D4704

[web] Align `MessageTimestampTooltip` under `MessageTooltip`
ClosedPublic
Actions

Authored by • abosh on Aug 1 2022, 1:00 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 2, 4:21 AM
Unknown Object (File)
Sun, Mar 30, 12:16 PM
Unknown Object (File)
Sat, Mar 15, 7:07 PM
Unknown Object (File)
Thu, Mar 13, 10:38 PM
Unknown Object (File)
Thu, Mar 13, 2:27 PM
Unknown Object (File)
Thu, Mar 13, 2:27 PM
Unknown Object (File)
Thu, Mar 13, 5:38 AM
Unknown Object (File)
Mon, Mar 10, 10:26 AM
View All Files
Subscribers
• adrian
ashoat

Details

Reviewers
atul
• jacek
tomek
Commits
rCOMMacd9728a0f6a: [web] Align `MessageTimestampTooltip` under `MessageTooltip`
Summary

Part of a stack of diffs to fix the timestamp bug, related Linear issue here.

This diff moves the timestamp tooltip below the message tooltip (with the reply and create thread buttons). Handling other front-end changes like color will be handled in subsequent diffs in this stack, this diff is just focused on aligning the tooltip correctly.

Depends on D4481.

Test Plan

Tested on Chrome/Safari and looks as expected.
Chrome:

Safari:

Additional stills:

image.png (240×578 px, 19 KB)

image.png (172×396 px, 16 KB)

image.png (350×1 px, 68 KB)

Diff Detail

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

Event Timeline

• abosh created this revision.Aug 1 2022, 1:00 PM
Herald added subscribers: • adrian, ashoat. · View Herald TranscriptAug 1 2022, 1:00 PM
• abosh edited the summary of this revision. (Show Details)Aug 1 2022, 1:02 PM
• abosh edited the test plan for this revision. (Show Details)
• abosh added a parent revision: D4481: [web] Reintroduce `MessageTimestampTooltip` in `ChatMessageList`.
• abosh added inline comments.Aug 1 2022, 1:07 PM
web/chat/message-timestamp-tooltip.react.js
22–24

Why were the values of these arrays swapped?

In the old tooltip logic, the tooltip for a composed viewer message, i.e., a message that you had written, would appear on the BOTTOM_RIGHT of the message:

image.png (176×268 px, 28 KB)

Similarly, in the old logic, the tooltip for a noncomposed or nonviewer message would appear on the LEFT of the message:

image.png (166×430 px, 19 KB)

However, as can be seen in the videos, this diff flips that logic to match prod. On prod, tooltips for composed viewer message appears on the LEFT of the message:

image.png (282×544 px, 27 KB)

And noncomposed message appears on the right:

image.png (300×1 px, 46 KB)

I kept it BOTTOM_RIGHT since I didn't want to break the old tooltip logic and introduce any regressions, but BOTTOM_RIGHT can probably be made RIGHT in the future.

• abosh attached a referenced file: F123492: image.png. (Show Details)Aug 1 2022, 1:07 PM
• abosh attached a referenced file: F123493: image.png. (Show Details)
• abosh attached a referenced file: F123494: image.png. (Show Details)
• abosh attached a referenced file: F123497: image.png. (Show Details)
Harbormaster completed remote builds in B11027: Diff 15168.Aug 1 2022, 1:11 PM
• abosh requested review of this revision.Aug 1 2022, 1:11 PM
• abosh added a child revision: D4705: [web] Finish styling `MessageTimestampTooltip`.Aug 1 2022, 1:48 PM
atul accepted this revision.Aug 4 2022, 8:47 AM
This revision is now accepted and ready to land.Aug 4 2022, 8:47 AM
Closed by commit rCOMMacd9728a0f6a: [web] Align `MessageTimestampTooltip` under `MessageTooltip` (authored by • abosh). · Explain WhyAug 4 2022, 10:45 AM
This revision was automatically updated to reflect the committed changes.
• abosh added a commit: rCOMMacd9728a0f6a: [web] Align `MessageTimestampTooltip` under `MessageTooltip`.

Revision Contents
Changeset List

PathSize
web/
chat/
21 lines

Diff 15168

View Options

web/chat/message-timestamp-tooltip.react.js

  • This file was added.
div.containerViewerLiked {
padding: 4px;
border-radius: 4px;
background-color: var(--shades-black-80);
}
div.container:hover {
cursor: pointer;
}
tomekUnsubmitted
Not Done
Inline Actions

For other tooltip actions we don't need to manually set this style. Can we use the mechanism we already have, or are there any reasons for not doing so?

tomek: For other tooltip actions we don't need to manually set this style. Can we use the mechanism we…
ginsuAuthorUnsubmitted
Done
Inline Actions

I believe the other icons don't need to set this style because it is built into IcomoonReact. When I switched the Like action tooltip button with one of the icons the hover style just works out of the box. Wondering if @atul has more context since he is pretty familiar with the icons stuff

ginsu: I believe the other icons don't need to set this style because it is built into `IcomoonReact`.
tomekUnsubmitted
Not Done
Inline Actions

It wouldn't make too much sense for Icomoon to provide hover effects for icons because they could be used in other places - not only as buttons. It seems like the style on hover is applied in message-tooltip.css

div.messageActionButtons svg:hover {
  cursor: pointer;
  color: var(--fg);
}

so it should be enough to apply this style also to the new button.

tomek: It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they…
ginsuAuthorUnsubmitted
Done
Inline Actions

Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes

ginsu: Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes