neynarxyz / react

React SDK for Farcaster components - powered by Neynar APIs
https://neynar-react.vercel.app
MIT License
18 stars 13 forks source link

[Optimization] Suggest stopPropagation when click reaction icons #12

Closed linrz closed 2 days ago

linrz commented 1 month ago

I am making some extended applications based on NeynarCastCard. For example, I need to open the corresponding Conversation when clicking NeynarCastCard. Is it better for us to add a line of event.stopPropagation() in the handleAction of src/components/atoms/Reactions.ts? Otherwise, clicking the icon of like/recast/comment will trigger the upper click event. The click event inside the component should be triggered only within the component. If you agree, I'd be happy to submit a PR to do it.

dylsteck commented 1 month ago

Hey @linrz! Happy to help here, I have two notes that come to mind that could help solve your problem

  1. If you're just looking to do something when the user clicks the component(eg. click a cast and then view its convo), you could wrap it in a parent tag like an a tag or a div with onClick properties.
  2. If you're looking to add your own functionality when a user reacts to a cast instead of our default functions, you can override any of the like/recast/comment icon click functions.
linrz commented 1 month ago

@dylsteck Thanks for your suggestion, maybe you misunderstood. I have already done this, my component structure is like this:

<div onClick={event => { navigateToConversation() }}>
  <NeynarCastCard />
</div>

Click Reactions inside the NeynarCastCard will trigger navigateToConversation

Even if I can distinguish Reactions by event.target in the onClick event of the div tag, its source is svg tag, but this is not elegant, so I want to suggest whether we should prevent events in onClick of Reactions icons.

Regarding the second point of your reply, only molecules/CastCard.tsx support like/recast/comment icon click functions, organisms/NeynarCastCard.tsx does not support click functions from props.

dylsteck commented 1 month ago

@dylsteck

Thanks for your suggestion, maybe you misunderstood. I have already done this, my component structure is like this:


<div onClick={event => { navigateToConversation() }}>

  <NeynarCastCard />

</div>

Click Reactions inside the NeynarCastCard will trigger navigateToConversation

Even if I can distinguish Reactions by event.target in the onClick event of the div tag, its source is svg tag, but this is not elegant, so I want to suggest whether we should prevent events in onClick of Reactions icons.

Regarding the second point of your reply, only molecules/CastCard.tsx support like/recast/comment icon click functions, organisms/NeynarCastCard.tsx does not support click functions from props.

Ah I see what you're saying -- my mistake, I had thought we moved those props to the parent component! In that case, if I quickly add those props to NeynarCastCard would that solve your problem? You'd then be able to override any of those functions and ensure nothing else happens(eg. stop event propagation). It's also worth nothing that if you don't want reactions at all, you could enable allowReactions to be false and the SVGs won't show up.

linrz commented 1 month ago

Of course, “add those props to NeynarCastCard” can solve the problem I encountered. I support this change. But I'm worried that what you do may cause trouble for developers who want to use Neynar's own Reactions built-in functions in the future, but also want to customize onClick in an external div like me.

dylsteck commented 2 days ago

Hey @linrz! I'm closing this comment as our latest version 0.9.0 has props where the developer can handle all click events both for cast interactions and for frame interactions, but let me know if anything else comes up 🙌