[web] modify useOnClickReact to return function with arguments instead of passing arguments directly into hook
ClosedPublic
Actions

Authored by ginsu on Jan 26 2023, 11:53 AM.

Details

Reviewers

atul
tomek

Commits

rCOMM2b64942985b2: [web] modify useOnClickReact to return function with arguments instead of…

Summary

modify useOnClickReact to return function with arguments instead of passing arguments directly into hook. This change will be necesary when we introduce the emoji keyboard, and we will need to pass the reactionInput through a useCallback hook

Depends on D6399
Linear Task: ENG-2776

Test Plan

flow, this will be further tested in a subsequent diff, and no regressions found when tested with plain message liking

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Jan 26 2023, 11:53 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptJan 26 2023, 11:53 AM

ginsu added reviewers: atul, tomek.Jan 26 2023, 11:54 AM

ginsu edited the summary of this revision. (Show Details)Jan 26 2023, 12:04 PM

ginsu added a parent revision: D6399: [web] introduce renderEmojiKeyboard and setRenderEmojiKeyboard to tooltip-provider.

Harbormaster completed remote builds in B15683: Diff 21385.Jan 26 2023, 12:07 PM

ginsu requested review of this revision.Jan 26 2023, 12:07 PM

ginsu added a child revision: D6402: [web] switch like button to react button on message tooltip.Jan 26 2023, 12:12 PM

Would we be able to pass all the arguments directly to the hook except the reactionInput? Personally feel like that would be a cleaner API

web/chat/reaction-message-utils.js
40	Is it okay to remove the `preventDefault()` here? Doesn't look like it's being used elsewhere.

atul accepted this revision.Jan 27 2023, 1:12 PM

This revision is now accepted and ready to land.Jan 27 2023, 1:12 PM

address atul's comments

web/chat/reaction-message-utils.js
40	Yea, it's not being used elsewhere, and it will be irrelevant in the next diff in this stack

Would we be able to pass all the arguments directly to the hook except the reactionInput?

Was able to pass all the arguments directly except for action. This is because action uses reactionInput to figure out whether to add_reaction or remove_reaction so they need to be together