Page MenuHomePhabricator
Differential D6452

[native] introduce get reactionSelectionPopoverLocation
AbandonedPublic
Actions

Authored by ginsu on Jan 29 2023, 8:22 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 1, 11:56 AM
Unknown Object (File)
Fri, Nov 1, 11:56 AM
Unknown Object (File)
Fri, Nov 1, 11:32 AM
Unknown Object (File)
Sat, Oct 26, 9:37 PM
Unknown Object (File)
Sep 30 2024, 10:23 AM
Unknown Object (File)
Sep 30 2024, 10:23 AM
Unknown Object (File)
Sep 30 2024, 10:19 AM
Unknown Object (File)
Sep 27 2024, 7:59 PM
View All 47 Files
Subscribers
ashoat
atul
tomek

Details

Reviewers
atul
tomek
Summary

introduce get reactionSelectionPopoverLocation. This will be used to determine if the reaction selection popover should be placed above or below the message

Depends on D6448
Linear Task: ENG-2779

Test Plan

flow and this will be further tested in a subsequent diff

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu created this revision.Jan 29 2023, 8:22 PM
Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptJan 29 2023, 8:22 PM
ginsu edited the summary of this revision. (Show Details)Jan 29 2023, 8:24 PM
ginsu added reviewers: atul, tomek.
ginsu added a parent revision: D6448: [native] introduce canCreateReactionFromMessage prop to Tooltip component.
ginsu edited the summary of this revision. (Show Details)
ginsu edited the summary of this revision. (Show Details)
ginsu added a child revision: D6453: [native] introduce useReactionSelectionPopoverPosition hook.Jan 29 2023, 8:30 PM
Harbormaster completed remote builds in B15800: Diff 21546.Jan 29 2023, 8:36 PM
ginsu requested review of this revision.Jan 29 2023, 8:36 PM
tomek requested changes to this revision.Jan 30 2023, 4:59 AM

I'm not really sure what's the plan here, probably it will be clarified later, but at this point it seems like we're hacking the solution by introducing something really similar to the tooltip. It feels like a better solution might be to extract the common part and use it inside our old tooltip and reaction tooltip, but I'm not yet sure.

native/navigation/tooltip.react.js
311

This function is really similar to tooltipLocation. What is the reason behind introducing it?

This revision now requires changes to proceed.Jan 30 2023, 4:59 AM
ginsu abandoned this revision.Jan 31 2023, 12:17 PM

This diff is no longer relevant since we don't want reactionSelectionPopoverLocation in the tooltip component

Revision Contents
Changeset List

PathSize
native/
navigation/
25 lines

Diff 21546

View Options

native/navigation/tooltip.react.js

Loading...