Display a popover when hovering over highlighted mentions #6820

acelaya · 2025-02-11T11:06:22Z

Depends on #6818

Display a popover with basic user information when hovering over a highlighted valid mention, whether it is a link or not.

Additionally, display a popover indicating a user does not exist when hovering over an invalid mention.

Popovers appear with a 400ms delay, to avoid a lot of flickering when hovering over annotations.

popover-with-delay-2025-02-12_12.35.53.mp4

src/sidebar/components/MarkdownView.tsx

codecov · 2025-02-11T11:08:32Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.42%. Comparing base (1e1103f) to head (bcbe3ae).
Report is 9 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6820      +/-   ##
==========================================
- Coverage   99.45%   99.42%   -0.03%     
==========================================
  Files         271      272       +1     
  Lines       10409    10451      +42     
  Branches     2488     2503      +15     
==========================================
+ Hits        10352    10391      +39     
- Misses         57       60       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

robertknight · 2025-02-12T10:19:54Z

The hover card currently appears immediately when a user's name is hovered, causing a flash if the cursor just happens to be passing over the mention. On GitHub, Reddit etc. to avoid this there is typically a short delay (100ms? I haven't measured it exactly) after the cursor enters the mention before the card is shown.

acelaya · 2025-02-12T10:23:20Z

The hover card currently appears immediately when a user's name is hovered, causing a flash if the cursor just happens to be passing over the mention. On GitHub, Reddit etc. to avoid this there is typically a short delay (100ms? I haven't measured it exactly) after the cursor enters the mention before the card is shown.

Yeah, good idea.

acelaya · 2025-02-12T11:38:31Z

src/sidebar/components/MarkdownView.tsx

+      if (content === null) {
+        setContent();
+      } else {
+        popoverContentTimeout.current = setTimeout_(setContent, 400);


Popovers appear after hovering for 400ms. I compared the time it takes in GitHub, and this is the value I came up "by eye", because I wasn't able to find a way to properly measure it.

robertknight · 2025-02-12T11:46:15Z

GitHub conveniently publish sourcemaps and their unminified source code in a form that is accessible to the debugger, so you can find the timeout they use:

hovercards.ts:

So you were correct with a 400ms timeout. I'm surprised, I thought this would be shorter.

acelaya · 2025-02-12T13:01:00Z

So you were correct with a 400ms timeout. I'm surprised, I thought this would be shorter.

I started with 100, then I realized it was showing much faster than in GitHub, so I tried with 200, then 400. At that point it felt close enough, but it took me a couple attempts 😅

acelaya · 2025-02-12T13:45:11Z

src/sidebar/components/MarkdownView.tsx

        <div
          className={classes}
          data-testid="markdown-text"
          ref={content}
          dangerouslySetInnerHTML={{ __html: html }}
          style={style}
+          // React types do not define `onMouseEnterCapture`, but preact does
+          // eslint-disable-next-line react/no-unknown-property


Disabling react/no-unknown-property here is needed because it evaluates props based on react types, not preact ones, where the Capture variants of event handlers are not defined.

We discussed in slack the possibility of completely disabling react/no-unknown-property, since TypeScript covers the same, more accurately, but I will do that on a PR of its own.

acelaya · 2025-02-12T15:13:09Z

src/sidebar/components/mentions/MentionPreviewContent.tsx

This contents are very much temporary, because matching the designs requires some extra API changes.

I'm defining a separate component, so that's easier to extend it and its test as soon as we have the missing pieces.

acelaya · 2025-02-12T15:15:58Z

Codecov is complaining about two lines that are marked with istanbul ignore next. This has happened in the past, not sure why.

robertknight · 2025-02-13T09:58:20Z

I investigated the default argument coverage issue and filed istanbuljs/istanbuljs#812 upstream. I found a workaround which I documented here: istanbuljs/istanbuljs#812 (comment)

robertknight

This looks good. I provided a workaround for the code coverage issue and some minor suggestions. One behavioral change I think we should make in future is that the popover should not disappear if the mouse leaves the mention element and moves on to the popover. This will enable adding links etc. in the popover, but also I think hiding the popover if the mouse moves into it is simply a little surprising.

src/sidebar/components/Annotation/AnnotationBody.tsx

src/sidebar/components/MarkdownView.tsx

robertknight · 2025-02-13T10:23:37Z

src/sidebar/components/MarkdownView.tsx

+  );
+  const [popoverContent, doSetPopoverContent] = useState<PopoverContent>(null);
+  const popoverContentTimeout = useRef<ReturnType<typeof setTimeout> | null>();
+  const setPopoverContent = useCallback(


There is nothing to clear the timer if the component gets unmounted after a hover has started but before the popover has been shown. You could use a layout effect cleanup function to handle this. A completely different approach would be to use an animation on the popover container to cause the delayed appearance (ie. the 400ms delay would become a transition on eg. visibility).

I'll add the effect cleanup function and check how difficult would be to use a native animation afterwards.

src/sidebar/components/MarkdownView.tsx

src/sidebar/components/mentions/MentionPreviewContent.tsx

src/sidebar/components/test/MarkdownView-test.js

acelaya · 2025-02-13T10:44:42Z

One behavioral change I think we should make in future is that the popover should not disappear if the mouse leaves the mention element and moves on to the popover. This will enable adding links etc. in the popover, but also I think hiding the popover if the mouse moves into it is simply a little surprising.

Yeah, I have been thinking about this. I think it should not be super hard to do, but requires some testing.

If at some point we add a gap between the anchor element and the popover, we may have to also set a delay to the popover hiding.

I'll create an issue with some notes on the most important considerations.

EDIT: #6821

acelaya commented Feb 11, 2025

View reviewed changes

src/sidebar/components/MarkdownView.tsx Outdated Show resolved Hide resolved

acelaya force-pushed the highlight-mentions branch 3 times, most recently from bc353c1 to 2f22ac3 Compare February 11, 2025 13:47

acelaya force-pushed the mentions-popover branch 2 times, most recently from 21c7577 to 2b48087 Compare February 11, 2025 13:55

acelaya force-pushed the highlight-mentions branch from 2f22ac3 to c989091 Compare February 12, 2025 09:27

Base automatically changed from highlight-mentions to main February 12, 2025 09:29

acelaya force-pushed the mentions-popover branch from 2b48087 to 2616ac2 Compare February 12, 2025 09:30

acelaya force-pushed the mentions-popover branch 2 times, most recently from 116dfb1 to 79e9832 Compare February 12, 2025 11:35

acelaya commented Feb 12, 2025

View reviewed changes

acelaya force-pushed the mentions-popover branch 3 times, most recently from ed351df to 306ee40 Compare February 12, 2025 13:42

acelaya commented Feb 12, 2025

View reviewed changes

acelaya force-pushed the mentions-popover branch 8 times, most recently from 5d44c1a to 1bd3f9c Compare February 12, 2025 15:12

acelaya commented Feb 12, 2025

View reviewed changes

acelaya marked this pull request as ready for review February 12, 2025 15:16

acelaya requested a review from robertknight February 12, 2025 15:16

robertknight approved these changes Feb 13, 2025

View reviewed changes

acelaya mentioned this pull request Feb 13, 2025

Keep the mention popover open when hovering over it #6821

Open

acelaya force-pushed the mentions-popover branch 2 times, most recently from 0a0602c to 98a0390 Compare February 13, 2025 11:01

Display a popover when hovering over highlighted mentions

bcbe3ae

acelaya force-pushed the mentions-popover branch from 98a0390 to bcbe3ae Compare February 13, 2025 11:07

acelaya merged commit 637be04 into main Feb 13, 2025
4 checks passed

acelaya deleted the mentions-popover branch February 13, 2025 11:11

acelaya mentioned this pull request Feb 13, 2025

Implement contents of the mention popover #6822

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display a popover when hovering over highlighted mentions #6820

Display a popover when hovering over highlighted mentions #6820

acelaya commented Feb 11, 2025 •

edited

Loading

codecov bot commented Feb 11, 2025 •

edited

Loading

robertknight commented Feb 12, 2025

acelaya commented Feb 12, 2025

acelaya Feb 12, 2025

robertknight commented Feb 12, 2025

acelaya commented Feb 12, 2025

acelaya Feb 12, 2025 •

edited

Loading

acelaya Feb 12, 2025

acelaya commented Feb 12, 2025

robertknight commented Feb 13, 2025

robertknight left a comment

robertknight Feb 13, 2025

acelaya Feb 13, 2025

acelaya commented Feb 13, 2025 •

edited

Loading

Display a popover when hovering over highlighted mentions #6820

Display a popover when hovering over highlighted mentions #6820

Conversation

acelaya commented Feb 11, 2025 • edited Loading

codecov bot commented Feb 11, 2025 • edited Loading

Codecov Report

robertknight commented Feb 12, 2025

acelaya commented Feb 12, 2025

acelaya Feb 12, 2025

Choose a reason for hiding this comment

robertknight commented Feb 12, 2025

acelaya commented Feb 12, 2025

acelaya Feb 12, 2025 • edited Loading

Choose a reason for hiding this comment

acelaya Feb 12, 2025

Choose a reason for hiding this comment

acelaya commented Feb 12, 2025

robertknight commented Feb 13, 2025

robertknight left a comment

Choose a reason for hiding this comment

robertknight Feb 13, 2025

Choose a reason for hiding this comment

acelaya Feb 13, 2025

Choose a reason for hiding this comment

acelaya commented Feb 13, 2025 • edited Loading

acelaya commented Feb 11, 2025 •

edited

Loading

codecov bot commented Feb 11, 2025 •

edited

Loading

acelaya Feb 12, 2025 •

edited

Loading

acelaya commented Feb 13, 2025 •

edited

Loading