fix: Hovercards are not displaying on the Contributor Distribution chart

[SURAJ-SHARMA27]

Description

This PR adds a custom tooltip feature to the NivoScatterPlot component. By integrating a hover card, the tooltip now displays detailed information about contributors when hovering over scatter plot nodes.

This PR fixes #3970

Related Tickets & Documents

Fixes #3970

Mobile & Desktop Screenshots/Recordings

screen-capture.39.webm

Steps to QA

1. Go to a repo page contributors
2. Hover over an avatar

Tier (staff will fill in)

• Tier 1
• Tier 2
• Tier 3
• Tier 4

[optional] What gif best describes this PR or how it makes you feel?

[netlify]

✅ Deploy Preview for oss-insights ready!

Name	Link
🔨 Latest commit	8fac751
🔍 Latest deploy log	https://app.netlify.com/sites/oss-insights/deploys/66e135009c4a5700084f0c81
😎 Deploy Preview	https://deploy-preview-4086--oss-insights.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[netlify]

✅ Deploy Preview for design-insights ready!

Name	Link
🔨 Latest commit	8fac751
🔍 Latest deploy log	https://app.netlify.com/sites/design-insights/deploys/66e13500ee7c7a00083ef64f
😎 Deploy Preview	https://deploy-preview-4086--design-insights.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[nickytonline]

@zeucapua and I had discussed a custom tooltip, but had been pulled onto other priorities. The issue is assigned to the core team, but this PR looks promising. I'll go ahead and assign it to you, but again, please read the contributing guidelines. You need to be assigned to an issue before working on it.

[SURAJ-SHARMA27]

Apologies, @nickytonline . Next time, I’ll make sure to ask for the issue to be assigned before working on it.

[nickytonline]

Hi @SURAJ-SHARMA27. Thanks for the PR. I've done some initial testing and there's a few issues:

• When mouseLeave fires, the tooltip remains. It should hide
• When hovering over an avatar, there is a constant flicker of the image icons. This suggests it's rerendering constantly for some reason. Maybe it's the position of the tooltip constantly being recalculated causing it.
• Only onMouseEnter and onMouseLeave should matter.

[SURAJ-SHARMA27]

@nickytonline Thanks for reviewing.
I resolved the issues. During debugging, I discovered that the mouse enter/leave events were working perfectly, but the main issue was with the useSupabaseAuth utility. This utility was being called repeatedly to retrieve the same data. I attempted to pass the ID from the parent component and only call useSupabaseAuth when the prop was not present, but this violated ESLint rules for calling React hooks. Making changes to that component seemed likely to disrupt other functionalities, so I created a custom tooltip component instead. It is working properly now. You can review it.

screen-capture.42.webm

[nickytonline]

Thanks for making some updates @SURAJ-SHARMA27. I see that it only appears on hover now and hides on mouseLeave. It works fine when logged in, but when logged out, all the hover cards say No Contributor Found.

In production, hover cards anywhere in the app work whether you're logged in or not.

CleanShot.2024-09-10.at.08.54.06.mp4

Also, looking at the new CustomTooltip, it looks like you're recreating the HoverCardWrapper which is something we don't want to be doing.

[SURAJ-SHARMA27]

thanks @nickytonline for reviewing.

It is now working for logged-out users as well. As I mentioned previously, I created a CustomTooltip component to address re rendering issue. In this component, useSupabaseAuth is not called for every hover trigger. Instead, it is called once in the parent component (nivo-scatter-plot), which resolves the re-rendering issue.

I cannot conditionally call hooks in the existing HoverCard component due to ESLint rules prohibiting conditional hook calls. Instead, a alternative is that we modify the parent components where the HoverCard is used and pass the userId as a prop.

screen-capture.43.webm

[nickytonline]

I had some refactoring suggestions, but I just realized there's an issue with the whole premise of using a Tooltip. Yes it appears when you hover over an avatar, but you can't interact with it because as soon as you try to hover over it, it closes because you've left node (Avatar) and mouseLeave fires.

I think we'll still need to get it working using the existing AvatarHovercard. Also, for reference, I've tried z-indez, absolute positioning, isolate, all kinds of things with no luck. That said, if we get the AvatarHovercard to open but do it in a way that mimics the CSS of the custom tooltip, I think we might be good, but again, you can't use the tooltip prop for reasons mentioned above.

[SURAJ-SHARMA27]

Yes, I researched a lot, but onClick events are not supported on tooltips of nivo-scatter-plot. I also tried delaying the mouseLeave event by 2 seconds to keep the tooltip visible so that we can click on it. However, the tooltip remains unclickable. I attempted to put the logic in handleClick, but it only handles redirection based on the hover avatar, not the redirection to the PR made by that user.

I also tried creating a custom div to display the avatarHoverCard, and it somewhat works, but I am dynamically updating its position based on client.X and client.Y. In some cases, this approach does not work well.

Should I try a different library?

[nickytonline]

Yes, I researched a lot, but onClick events are not supported on tooltips of nivo-scatter-plot. I also tried delaying the mouseLeave event by 2 seconds to keep the tooltip visible so that we can click on it. However, the tooltip remains unclickable. I attempted to put the logic in handleClick, but it only handles redirection based on the hover avatar, not the redirection to the PR made by that user.

I also tried creating a custom div to display the avatarHoverCard, and it somewhat works, but I am dynamically updating its position based on client.X and client.Y. In some cases, this approach does not work well.

Should I try a different library?

At the moment we probably aren't looking to switch charts. I had tried a Recharts scatter chart briefly since we use those, but didn't have enough time to thoroughly investigate.

I think it's still possible with the actual hover card as long as the hovercard wrapper renders outside the <foreignobject>. I just haven't had time to try that myself. We wouldn't use our dialog component, but maybe a React portal on the document.body.

Here's is a small proof of concept of it working.

/* eslint-disable @next/next/no-img-element */
import * as HoverCard from "@radix-ui/react-hover-card";
import Link from "next/link";

import Image from "next/image";
import HoverCardWrapper from "components/molecules/HoverCardWrapper/hover-card-wrapper";

import { getAvatarByUsername } from "lib/utils/github";
import { createPortal } from "react-dom";
import { useState } from "react";
import { is } from "valibot";

declare interface AvatarProps {
  contributor: string;
  size?: "xxsmall" | "xsmall" | "small" | "medium" | "large";
}

type AvatarHoverCard = AvatarProps & {
  repositories: number[];
};

export const Avatar = ({ contributor, size = "large" }: AvatarProps): JSX.Element => {
  let width = 500;
  let height = 500;

  switch (size) {
    case "xxsmall":
      width = 18;
      height = 18;
      break;
    case "xsmall":
      width = 24;
      height = 24;
      break;
    case "small":
      width = 45;
      height = 45;
      break;

    case "medium":
      width = 35;
      height = 35;
      break;
    default:
      break;
  }

  return (
    <Image
      alt={contributor}
      className="border rounded-full not-prose"
      height={width}
      src={getAvatarByUsername(contributor, 40)}
      width={height}
    />
  );
};

const AvatarHoverCard = ({ contributor, repositories, size = "large" }: AvatarHoverCard): JSX.Element => {
  const [isHovered, setIsHovered] = useState(false);

  if (isHovered) {
    console.log("isHovered", isHovered);
  }

  return (
    <>
      <Link
        href={`/u/${contributor}`}
        as={`/u/${contributor}`}
        onMouseEnter={() => setIsHovered(true)}
        onMouseLeave={() => setIsHovered(false)}
      >
        <Avatar contributor={contributor} size={size} />
      </Link>

      {isHovered
        ? createPortal(
            <div
              className="absolute"
              style={{
                top: "0",
                left: "0",
                width: "400",
                height: "400",
              }}
            >
              <HoverCardWrapper username={contributor} repositories={repositories} />
            </div>,
            document.body
          )
        : null}
    </>
  );
};

export default AvatarHoverCard;

The useSupabaseAuth() causes the flickering but once removed it works, but I think that should be passed in as a loggedIn prop. Hope that helps.

[SURAJ-SHARMA27]

At the moment we probably aren't looking to switch charts. I had tried a Recharts scatter chart briefly since we use those, but didn't have enough time to thoroughly investigate.

I think it's still possible with the actual hover card as long as the hovercard wrapper renders outside the <foreignobject>. I just haven't had time to try that myself. We wouldn't use our dialog component, but maybe a React portal on the document.body.

Here's is a small proof of concept of it working.
...

I was trying these approaches only and I mentioned in my previous messages too :

1. We can use a separate div to render our AvatarHoverCard component, but the challenge was adjusting its position dynamically for each node. Earlier, I tried using event.clientX and event.clientY to achieve this.

2. As I mentioned before, useSupabaseAuth() retrieves the same info for each hover, causing flickering. Instead, we can pass that same prop from the parent component to handle the flickering. However, since the useSupabaseAuth() hook is called inside the AvatarHoverCard component, and according to ESLint rules, we cannot render a hook conditionally. That's why I created a custom tooltip component where useSupabaseAuth() is not called, and the necessary information is passed from the parent component.

3. I worked on the separate div approach and made progress. Now, the tooltip is interactive and works perfectly within the same chart. However, I’ve noticed one issue: sometimes the mouseleave event doesn't work properly. And i think it's not the right way to render things based on clientX and Y coordinates.

screen-capture.44.webm

[nickytonline]

Just seeing if you are still pursuing this PR @SURAJ-SHARMA27?

[nickytonline]

I'm gpoing to go ahead and close this for now. Feel free to reopen it if you want to continue working on this @SURAJ-SHARMA27.