knocklabs / javascript

Official JavaScript packages for interacting with Knock
https://knock.app/
MIT License
16 stars 4 forks source link

Conflicting Event Handlers #191

Open cookrn opened 3 months ago

cookrn commented 3 months ago

Hello! While following the default instructions for rendering a notification feed in my Next.js app, I ran into issues with the event handler for closing the popover when clicking on the bell icon.

It looks to be an issue where the popover onClose was firing first to set the isVisible state to false and then the button onClick was firing second, updating the isVisible state back to true. The result was that the popover would not toggle closed when clicking the 🔔, only when clicking on the document outside of the popover.

I solved it with the following changes:

'use client';

import React, { useState, useRef } from 'react';
import { KnockProvider, KnockFeedProvider, NotificationIconButton, NotificationFeedPopover } from '@knocklabs/react';

// Required CSS import to use default Knock styles
import '@knocklabs/react/dist/index.css';

export default function KnockNotificationClientIcon({ userId, userToken }: { userId: string; userToken: string }) {
  const [isVisible, setIsVisible] = useState(false);
  const notifButtonRef = useRef<HTMLButtonElement>(null);

  function handlePopoverClose(e: Event) {
    if (e.target instanceof Element && !notifButtonRef.current?.contains(e.target)) {
      setIsVisible(false);
    }
  }

  return (
    <KnockProvider apiKey={process.env.NEXT_PUBLIC_KNOCK_PUBLIC_API_KEY} userId={userId} userToken={userToken}>
      <KnockFeedProvider feedId={process.env.NEXT_PUBLIC_KNOCK_FEED_CHANNEL_ID}>
        <NotificationIconButton ref={notifButtonRef} onClick={() => setIsVisible(!isVisible)} />
        <NotificationFeedPopover buttonRef={notifButtonRef} isVisible={isVisible} onClose={handlePopoverClose} />
      </KnockFeedProvider>
    </KnockProvider>
  );
}

Note the onClose handler for the popover now detects if the clicked element is a child of the notification button. Have you noticed or had other reports of this? Is there somewhere I could submit a PR for the docs?

Cheers 🍻

niss36 commented 1 month ago

Thanks for the workaround, also facing the same problem!