Details

Reviewers

atul
rohan
ashoat

Commits

rCOMMdf2ae6258384: [native] changed tooltip toolbar color from light to dark

Summary

changed color of all tooltips from light to dark, as discussed in this Linear comment Will touch up animation, and add the more button for the fixed tooltips in a seperate diff

Depends on D5451
Linear Task: ENG-2037
Design: Figma

Test Plan

Please view the video demo to see how all the tooltips now look in the new theme:

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Oct 21 2022, 1:10 PM

Herald added subscribers: • abosh, atul, tomek, ashoat. · View Herald TranscriptOct 21 2022, 1:10 PM

ginsu edited the summary of this revision. (Show Details)Oct 21 2022, 1:13 PM

ginsu edited the test plan for this revision. (Show Details)

ginsu added reviewers: atul, rohan.

ginsu added a parent revision: D5459: [native] moved tooltip toolbar to the bottom of the chat window.

ginsu attached a referenced file: F205905: Screen Shot 2022-10-21 at 4.08.19 PM.png. (Show Details)

ginsu attached a referenced file: F205904: Screen Shot 2022-10-21 at 4.10.59 PM.png. (Show Details)

Harbormaster completed remote builds in B12978: Diff 17828.Oct 21 2022, 1:21 PM

ginsu requested review of this revision.Oct 21 2022, 1:21 PM

Looks great

native/navigation/tooltip.react.js
457 ↗	(On Diff #17828)	I'd personally prefer if we used hex for colors throughout
471 ↗	(On Diff #17828)	Can we capitalize the `f`s here?

This revision is now accepted and ready to land.Oct 21 2022, 2:17 PM

rohan accepted this revision.Oct 21 2022, 4:32 PM

rebase

Harbormaster completed remote builds in B12995: Diff 17854.Oct 24 2022, 8:41 AM

Harbormaster completed remote builds in B13000: Diff 17859.Oct 24 2022, 8:45 AM

rebase

Harbormaster failed remote builds in B13003: Diff 17862!Oct 24 2022, 9:31 AM

Can you share screenshots of how this looks for all of the other tooltips? High-level feedback – whenever updating the Tooltip code, please make sure you consider all codepaths that will impact... we should see other codepaths referenced in the Test Plan and in screenshots

(Looks great though!!)

rebase

In D5460#161644, @ashoat wrote:

Can you share screenshots of how this looks for all of the other tooltips? High-level feedback – whenever updating the Tooltip code, please make sure you consider all codepaths that will impact... we should see other codepaths referenced in the Test Plan and in screenshots

Gotcha, I will make sure to be very explicit about this in the test cases in the future

Harbormaster completed remote builds in B13003: Diff 17862.Oct 24 2022, 10:38 AM

Harbormaster completed remote builds in B13016: Diff 17875.

Missing tooltips

This revision now requires changes to proceed.Oct 24 2022, 10:59 AM