Skip to content

Commit

Permalink
Highlight mentions when rendering annotation text
Browse files Browse the repository at this point in the history
  • Loading branch information
acelaya committed Feb 11, 2025
1 parent eca9044 commit bc353c1
Show file tree
Hide file tree
Showing 9 changed files with 70 additions and 9 deletions.
1 change: 1 addition & 0 deletions src/sidebar/components/Annotation/AnnotationBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ function AnnotationBody({ annotation, settings }: AnnotationBodyProps) {
'p-redacted-text': isHidden(annotation),
})}
style={textStyle}
mentions={annotation.mentions}
/>
</Excerpt>
)}
Expand Down
1 change: 1 addition & 0 deletions src/sidebar/components/Annotation/AnnotationEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ function AnnotationEditor({
mentionsEnabled={mentionsEnabled}
usersForMentions={usersWhoAnnotated}
showHelpLink={showHelpLink}
mentions={annotation.mentions}
/>
<TagEditor
onAddTag={onAddTag}
Expand Down
6 changes: 6 additions & 0 deletions src/sidebar/components/MarkdownEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
} from 'preact/hooks';

import { isMacOS } from '../../shared/user-agent';
import type { Mention } from '../../types/api';
import {
getContainingMentionOffsets,
termBeforePosition,
Expand Down Expand Up @@ -547,6 +548,9 @@ export type MarkdownEditorProps = {
* this list.
*/
usersForMentions: UserItem[];

/** List of mentions extracted from the annotation text. */
mentions?: Mention[];
};

/**
Expand All @@ -560,6 +564,7 @@ export default function MarkdownEditor({
textStyle = {},
showHelpLink = true,
usersForMentions,
mentions,
}: MarkdownEditorProps) {
// Whether the preview mode is currently active.
const [preview, setPreview] = useState(false);
Expand Down Expand Up @@ -611,6 +616,7 @@ export default function MarkdownEditor({
markdown={text}
classes="border bg-grey-1 p-2"
style={textStyle}
mentions={mentions}
/>
) : (
<TextArea
Expand Down
8 changes: 8 additions & 0 deletions src/sidebar/components/MarkdownView.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { useEffect, useMemo, useRef } from 'preact/hooks';

import type { Mention } from '../../types/api';
import { renderMentionTags } from '../helpers/mentions';
import { replaceLinksWithEmbeds } from '../media-embedder';
import { renderMathAndMarkdown } from '../render-markdown';
import StyledText from './StyledText';
Expand All @@ -9,6 +11,7 @@ export type MarkdownViewProps = {
markdown: string;
classes?: string;
style?: Record<string, string>;
mentions?: Mention[];
};

/**
Expand All @@ -19,6 +22,7 @@ export default function MarkdownView({
markdown,
classes,
style,
mentions,
}: MarkdownViewProps) {
const html = useMemo(
() => (markdown ? renderMathAndMarkdown(markdown) : ''),
Expand All @@ -35,6 +39,10 @@ export default function MarkdownView({
});
}, [markdown]);

useEffect(() => {
renderMentionTags(content.current!, mentions ?? []);
}, [mentions]);

// NB: The following could be implemented by setting attribute props directly
// on `StyledText` (which renders a `div` itself), versus introducing a child
// `div` as is done here. However, in initial testing, this interfered with
Expand Down
13 changes: 13 additions & 0 deletions src/sidebar/components/test/MarkdownView-test.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
import { checkAccessibility } from '@hypothesis/frontend-testing';
import { mount } from '@hypothesis/frontend-testing';
import sinon from 'sinon';

import MarkdownView, { $imports } from '../MarkdownView';

describe('MarkdownView', () => {
let fakeRenderMathAndMarkdown;
let fakeReplaceLinksWithEmbeds;
let fakeRenderMentionTags;

const markdownSelector = '[data-testid="markdown-text"]';

beforeEach(() => {
fakeRenderMathAndMarkdown = markdown => `rendered:${markdown}`;
fakeReplaceLinksWithEmbeds = sinon.stub();
fakeRenderMentionTags = sinon.stub();

$imports.$mock({
'../render-markdown': {
Expand All @@ -20,6 +23,9 @@ describe('MarkdownView', () => {
'../media-embedder': {
replaceLinksWithEmbeds: fakeReplaceLinksWithEmbeds,
},
'../helpers/mentions': {
renderMentionTags: fakeRenderMentionTags,
},
});
});

Expand Down Expand Up @@ -69,6 +75,13 @@ describe('MarkdownView', () => {
});
});

[undefined, [{}]].forEach(mentions => {
it('renders mention tags based on provided mentions', () => {
mount(<MarkdownView mentions={mentions} />);
assert.calledWith(fakeRenderMentionTags, sinon.match.any, mentions ?? []);
});
});

it(
'should pass a11y checks',
checkAccessibility({
Expand Down
14 changes: 8 additions & 6 deletions src/sidebar/helpers/mentions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,12 @@ function elementForMention(
mentionLink: HTMLElement,
mention?: Mention,
): [HTMLElement, Mention | InvalidUsername] {
// If the mention exists in the list of mentions, render it as a link
// If the mention exists in the list of mentions and contains a link, render
// it as an anchor pointing to that link
if (mention && mention.link) {
mentionLink.setAttribute('href', mention.link ?? '');
mentionLink.setAttribute('href', mention.link);
mentionLink.setAttribute('target', '_blank');
mentionLink.classList.add('font-bold');
mentionLink.setAttribute('data-hyp-mention-type', 'link');

return [mentionLink, mention];
}
Expand All @@ -73,9 +74,9 @@ function elementForMention(
if (!mention) {
const invalidMention = document.createElement('span');

invalidMention.setAttribute('data-hyp-mention', '');
invalidMention.setAttribute('data-hyp-mention-type', 'invalid');
invalidMention.textContent = username;
invalidMention.style.fontStyle = 'italic';
invalidMention.style.borderBottom = 'dotted';
mentionLink.replaceWith(invalidMention);

return [invalidMention, username];
Expand All @@ -86,9 +87,10 @@ function elementForMention(
const nonLinkMention = document.createElement('span');

nonLinkMention.setAttribute('data-hyp-mention', '');
nonLinkMention.setAttribute('data-hyp-mention-type', 'no-link');
nonLinkMention.setAttribute('data-userid', mentionLink.dataset.userid ?? '');
nonLinkMention.classList.add('text-brand', 'font-bold');
nonLinkMention.textContent = username;
mentionLink.replaceWith(nonLinkMention);

return [nonLinkMention, mention];
}
Expand Down
10 changes: 7 additions & 3 deletions src/sidebar/helpers/test/mentions-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,21 +126,25 @@ describe('renderMentionTags', () => {
firstElement.getAttribute('href'),
'http://example.com/janedoe',
);
assert.equal(firstElement.dataset.hypMentionType, 'link');
assert.equal(firstMention, mentions[0]);

// Second element will render as a highlighted span
assert.equal(secondElement.tagName, 'SPAN');
assert.equal(secondElement.dataset.userid, 'acct:[email protected]');
assert.isTrue(secondElement.hasAttribute('data-hyp-mention'));
assert.equal(secondElement.dataset.hypMentionType, 'no-link');
assert.isTrue(secondElement.hasAttribute('data-userid'));
assert.equal(secondMention, mentions[1]);

// Third and fourth elements will be invalid mentions wrapping the invalid
// username
assert.equal(thirdElement.tagName, 'SPAN');
assert.isFalse(thirdElement.hasAttribute('data-hyp-mention'));
assert.isFalse(thirdElement.hasAttribute('data-userid'));
assert.equal(thirdElement.dataset.hypMentionType, 'invalid');
assert.equal(thirdMention, '@invalid');
assert.equal(fourthElement.tagName, 'SPAN');
assert.isFalse(fourthElement.hasAttribute('data-hyp-mention'));
assert.isFalse(fourthElement.hasAttribute('data-userid'));
assert.equal(fourthElement.dataset.hypMentionType, 'invalid');
assert.equal(fourthMention, '@user_id_missing');
});
});
Expand Down
20 changes: 20 additions & 0 deletions src/styles/sidebar/components/StyledText.scss
Original file line number Diff line number Diff line change
Expand Up @@ -99,5 +99,25 @@
blockquote {
@apply border-l-[3px] italic px-[1em] text-color-text-light;
}

// Un-processed mentions look like plain text
a[data-hyp-mention] {
@apply text-inherit no-underline;
}

// Valid processed mention with link
a[data-hyp-mention][data-hyp-mention-type="link"] {
@apply text-brand font-bold underline;
}

// Valid processed mention without link
span[data-hyp-mention][data-hyp-mention-type="no-link"] {
@apply text-brand font-bold;
}

// Invalid processed mention
span[data-hyp-mention][data-hyp-mention-type="invalid"] {
@apply text-grey-6 font-bold underline decoration-wavy decoration-red-error;
}
}
}
6 changes: 6 additions & 0 deletions src/types/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,12 @@ export type APIAnnotationData = {
* current assignment, course etc. to annotations.
*/
metadata?: object;

/**
* List of unique users that were mentioned in the annotation text.
* This prop will be present only if `at_mentions` is enabled.
*/
mentions?: Mention[];
};

/**
Expand Down

0 comments on commit bc353c1

Please sign in to comment.