[PoC] Support creating mentions as part of annotation creation #6787

acelaya · 2025-02-03T09:51:53Z

This PR is used to verify some assumptions, and check that we can proceed according to what's described in the engineering design document for phase 1 https://docs.google.com/document/d/16Iv-MPwQi_LIIN6Fk-aAaflZqWkSX5bQIWUEg_MUDBI/edit

The implementation here is very rough, and individual PRs will be extracted with the actual, properly tested implementation.

This PR has been tested in combination with hypothesis/h#9279, to ensure FE and BE integrate as expected.

highlighted-at-mention-2025-02-03_16.47.17.mp4

mentions-popover-2025-02-05_12.00.56.mp4

Logic

This PR covers:

Replacing any occurrence of @something in the annotation text with the special mention tag (<a data-hyp-mention data-userid="acct:something@authority">@something</a>) right before sending it to the BE.
We don't try to validate those are actual usernames client-side. Instead we let the BE validate them and "discard" those that are invalid.
This will allow us to highlight incorrect mentions to end-users, or simply render them as plain-text if desired.
Convert special mention tags to their plain-text representation when going back to editing an annotation.
Render valid mentions as a link to the user profile, and invalid ones as a styled span.
Display a popover when hovering over valid at-mentions. The content is temporary (display name or username), but we'll have to enhance it.

This PR assumes usernames are public and user-friendly. In future, we'll need to support mentioning via display name as well, for the LMS. In that case we will have to support an additional plain-text format for mentions that could be something like @[Display name], @<Display name>, @Display_name or something in those lines.

When converting that to the corresponding mention tag, we will have to get the userid somehow. Let's worry about that later.

TODO

Parse @username into special tag <a data-hyp-mention data-userid="acct:username@authority">@username</a>
Render valid mentions from server as links to the user profile, and invalid ones as plain-text or some fallback style
Parse special tag back into @username when entering annotation edit mode for an annotation with mentions
Show basic popover when hovering over a mention

codecov · 2025-02-03T09:54:28Z

Codecov Report

Attention: Patch coverage is 25.00000% with 51 lines in your changes missing coverage. Please review.

Project coverage is 98.94%. Comparing base (6a8e7ae) to head (1364a97).

Files with missing lines	Patch %	Lines
src/sidebar/helpers/mentions.ts	21.21%	26 Missing ⚠️
src/sidebar/components/MarkdownView.tsx	22.58%	24 Missing ⚠️
src/sidebar/services/annotations.ts	66.66%	1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (25.00%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6787      +/-   ##
==========================================
- Coverage   99.43%   98.94%   -0.49%     
==========================================
  Files         270      271       +1     
  Lines       10312    10379      +67     
  Branches     2474     2493      +19     
==========================================
+ Hits        10254    10270      +16     
- Misses         58      109      +51

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

src/sidebar/components/MarkdownEditor.tsx

acelaya · 2025-02-04T10:18:17Z

src/sidebar/components/MarkdownView.tsx

@@ -35,21 +76,49 @@ export default function MarkdownView({
    });
  }, [markdown]);

+  useEffect(() => {
+    const listenerCollection = new ListenerCollection();
+    const foundMentions = replaceMentionTags(content.current!, mentions);


Similarly to how we replace links with embeds via a side effect, we replace mention tags with their rendered representation directly in the DOM.

src/sidebar/components/MarkdownView.tsx

acelaya · 2025-02-04T10:20:13Z

src/sidebar/services/annotations.ts

+function mentionsToTags(text: string, authority: string): string {
+  return text.replace(/(?:^|\s)@(\w+)(?=\s|$)/g, (match, username) => {
+    const userid = `acct:${username}@${authority}`;
+    return ` <a data-hyp-mention data-userid="${userid}">@${username}</a>`;
+  });
+}


This converts plain-text mentions into the special tag, but assumes the mention is the username. We'll have to evolve this for display-name-based mentions.

src/sidebar/services/annotations.ts

acelaya · 2025-02-04T10:21:46Z

src/types/api.ts

+  userid: string;
+  username: string;
+  link: string;


This may not be entirely correct. For example, during my testing I was getting null links. We'll have to clarify this with the BE.

robertknight

I would suggest to extract the functions related to extracting and replacing mentions into a module in helpers where they can be tested directly. Also I would default to use browser APIs for parsing and generating HTML unless we determine they can't be used for some reason. For listening for events, I suggested to try event delegation rather than adding listeners to each mention directly - unless you can see a reason we can't do that?

src/sidebar/components/MarkdownEditor.tsx

src/sidebar/components/MarkdownView.tsx

src/sidebar/services/annotations.ts

src/types/api.ts

robertknight · 2025-02-04T12:15:24Z

src/types/api.ts

+export type Mention = UserInfo & {
+  userid: string;
+  username: string;
+  link: string;


What does link point to? The user's profile page? What about LMS users in future who don't have profile pages?

What does link point to? The user's profile page?

Yes, that is the intention

What about LMS users in future who don't have profile pages?

The field should be nullable. If it comes as null, then we would not render mentions as links, but something else.

robertknight · 2025-02-04T12:30:16Z

For unresolvable mentions, I think it would be helpful to have a popover to explicitly indicate what the styling means:

The text of the popover could be something like "No user with username foobar exists".

acelaya · 2025-02-04T13:09:39Z

Also I would default to use browser APIs for parsing and generating HTML unless we determine they can't be used for some reason.

Yep, this is a good suggestion, as using regular expressions proved to be problematic.

For some reason I've been working on the assumption that we would have to use regular expressions, so didn't really think on an alternative, but since we are dealing with HTML elements, using the browser APIs makes a lot of sense.

robertknight

This generally looks good. I had a query about handling of null links when post-processing the rendered content for an annotation.

robertknight · 2025-02-05T11:19:32Z

src/sidebar/components/MarkdownView.tsx

+    const listenerCollection = new ListenerCollection();
+    const foundMentions = renderMentionTags(content.current!, mentions);
+
+    listenerCollection.add(


Was there a reason not to use onMouseEnterCapture and onMouseLeaveCapture on the content div? (onMouseOver and onMouseOut could work as well with some other modifications I think).

The main reason is that renderMentionTags needs to be called in a side effect, so we would have to track the result in a ref.

Defining the event handlers here simplified that, as it no longer needs to be referenced outside. But we can explore the other option.

src/sidebar/components/MarkdownView.tsx

src/sidebar/helpers/mentions.ts

robertknight · 2025-02-05T11:28:07Z

src/sidebar/helpers/mentions.ts

+
+    if (mention) {
+      // If the mention exists in the list of mentions, render it as a link
+      mentionLink.setAttribute('href', mention.link);


I think you mentioned previously that link can be null? Is that correct? If so what should happen here?

yeah, that's correct. In that case this should be rendered as a different type of tag, and highlighted by color only, but not underlined.

src/types/api.ts

… tags

acelaya · 2025-02-05T14:31:24Z

This PoC has served its purpose. I'm closing it now and will start creating individual PRs

acelaya force-pushed the mention-tags-poc branch 5 times, most recently from 6dc1f40 to 37cd560 Compare February 4, 2025 09:38

acelaya commented Feb 4, 2025

View reviewed changes

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

acelaya commented Feb 4, 2025

View reviewed changes

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

acelaya commented Feb 4, 2025

View reviewed changes

src/sidebar/services/annotations.ts Outdated Show resolved Hide resolved

acelaya commented Feb 4, 2025

View reviewed changes

acelaya requested a review from robertknight February 4, 2025 10:29

robertknight reviewed Feb 4, 2025

View reviewed changes

acelaya mentioned this pull request Feb 4, 2025

Mentions PoC hypothesis/h#9279

Closed

acelaya force-pushed the mention-tags-poc branch from a748567 to 097f524 Compare February 5, 2025 11:14

robertknight reviewed Feb 5, 2025

View reviewed changes

acelaya added 11 commits February 5, 2025 15:26

Convert plain mentions into tag mentions when saving annotation

ae87370

Render mentions as links in annotation preview

04e0bef

Fix mention matching in text

a464645

Unwrap special mention tags when entering annotation edit mode

76a6437

Fix fallback authority when creating annotation mentions

9725fc8

Add basic popover for at-mentions

7bca966

Extract mention-related helpers to their own module and use DOM APIs

a6ca979

Resolve authority from currently logged-in user when building mention…

b80cab4

… tags

Extract logic to highlight mentions to the mentions module

01474ec

Use event delegation for the mentions popover

3acc145

Add a more complex mention popover content

3b9ed81

acelaya added 3 commits February 5, 2025 15:26

Display popover for invalid mentions

e9eb8ec

Fix JSDoc typo

ce48997

Add missing API type docs for mentions

1364a97

acelaya force-pushed the mention-tags-poc branch from 097f524 to 1364a97 Compare February 5, 2025 14:30

acelaya closed this Feb 5, 2025

acelaya mentioned this pull request Feb 5, 2025

Create helper functions to manipulate mentions #6797

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PoC] Support creating mentions as part of annotation creation #6787

[PoC] Support creating mentions as part of annotation creation #6787

acelaya commented Feb 3, 2025 •

edited

Loading

codecov bot commented Feb 3, 2025 •

edited

Loading

acelaya Feb 4, 2025 •

edited

Loading

acelaya Feb 4, 2025

acelaya Feb 4, 2025

robertknight left a comment

robertknight Feb 4, 2025

acelaya Feb 4, 2025

robertknight commented Feb 4, 2025

acelaya commented Feb 4, 2025

robertknight left a comment

robertknight Feb 5, 2025

acelaya Feb 5, 2025 •

edited

Loading

robertknight Feb 5, 2025

acelaya Feb 5, 2025

acelaya commented Feb 5, 2025

[PoC] Support creating mentions as part of annotation creation #6787

[PoC] Support creating mentions as part of annotation creation #6787

Conversation

acelaya commented Feb 3, 2025 • edited Loading

Logic

TODO

codecov bot commented Feb 3, 2025 • edited Loading

Codecov Report

acelaya Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

acelaya Feb 4, 2025

Choose a reason for hiding this comment

acelaya Feb 4, 2025

Choose a reason for hiding this comment

robertknight left a comment

Choose a reason for hiding this comment

robertknight Feb 4, 2025

Choose a reason for hiding this comment

acelaya Feb 4, 2025

Choose a reason for hiding this comment

robertknight commented Feb 4, 2025

acelaya commented Feb 4, 2025

robertknight left a comment

Choose a reason for hiding this comment

robertknight Feb 5, 2025

Choose a reason for hiding this comment

acelaya Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

robertknight Feb 5, 2025

Choose a reason for hiding this comment

acelaya Feb 5, 2025

Choose a reason for hiding this comment

acelaya commented Feb 5, 2025

acelaya commented Feb 3, 2025 •

edited

Loading

codecov bot commented Feb 3, 2025 •

edited

Loading

acelaya Feb 4, 2025 •

edited

Loading

acelaya Feb 5, 2025 •

edited

Loading