bot comments, hide popup when overlapping existing highlight

Signed-off-by: Rudransh Shrivastava <rudransh.shrivastava@owasp.org>

Author: rudransh-shrivastava

frontend/__tests__/unit/components/AnnotatedProfile.test.tsx

@@ -1,6 +1,7 @@
 import { fireEvent, render, screen } from '@testing-library/react'
 import { useProfileSelection } from 'hooks/useProfileSelection'
 import { useRouter } from 'next/navigation'
+import React from 'react'
 
 import { ClaimStatusEnum } from 'types/__generated__/graphql'
 import AnnotatedProfile from 'components/AnnotatedProfile'
@@ -9,6 +10,36 @@ jest.mock('hooks/useProfileSelection', () => ({
   useProfileSelection: jest.fn(() => null),
 }))
 
+jest.mock(
+  'components/ClaimHighlight',
+  () =>
+    function MockClaimHighlight({
+      children,
+      'data-claim-key': claimKey,
+      'data-claim-name': claimName,
+      'data-claim-status': claimStatus,
+      ...rest
+    }: {
+      children?: React.ReactNode
+      'data-claim-key'?: string
+      'data-claim-name'?: string
+      'data-claim-status'?: string
+    } & Record<string, unknown>) {
+      if (!claimKey) return <span {...rest}>{children}</span>
+      return (
+        <span
+          data-testid="claim-highlight"
+          data-claim-highlight="true"
+          data-claim-key={claimKey}
+          data-claim-name={claimName}
+          data-claim-status={claimStatus}
+        >
+          {children}
+        </span>
+      )
+    }
+)
+
 const mockUseProfileSelection = useProfileSelection as jest.Mock
 
 const baseProps = {
@@ -48,9 +79,11 @@ describe('AnnotatedProfile', () => {
         ]}
       />
     )
-    const link = screen.getByRole('link', { name: /Claim: A Claim, status Approved/i })
-    expect(link).toBeInTheDocument()
-    expect(link.textContent).toBe('This is my claim.')
+    const mark = screen.getByTestId('claim-highlight')
+    expect(mark).toHaveAttribute('data-claim-key', 'claim-key')
+    expect(mark).toHaveAttribute('data-claim-name', 'A Claim')
+    expect(mark).toHaveAttribute('data-claim-status', ClaimStatusEnum.Approved)
+    expect(mark.textContent).toBe('This is my claim.')
   })
 
   it('skips claims whose sourceText contains a blank line', () => {
@@ -69,7 +102,7 @@ describe('AnnotatedProfile', () => {
         ]}
       />
     )
-    expect(screen.queryByRole('link')).not.toBeInTheDocument()
+    expect(screen.queryByTestId('claim-highlight')).not.toBeInTheDocument()
   })
 
   it('drops claims whose sourceText is not found', () => {
@@ -88,7 +121,7 @@ describe('AnnotatedProfile', () => {
         ]}
       />
     )
-    expect(screen.queryByRole('link')).not.toBeInTheDocument()
+    expect(screen.queryByTestId('claim-highlight')).not.toBeInTheDocument()
   })
 
   it('gives longer sourceText priority when two claims overlap', () => {
@@ -114,9 +147,10 @@ describe('AnnotatedProfile', () => {
         ]}
       />
     )
-    const links = screen.getAllByRole('link')
-    expect(links).toHaveLength(1)
-    expect(links[0]).toHaveTextContent('Overlap area')
+    const marks = screen.getAllByTestId('claim-highlight')
+    expect(marks).toHaveLength(1)
+    expect(marks[0]).toHaveTextContent('Overlap area')
+    expect(marks[0]).toHaveAttribute('data-claim-key', 'long')
   })
 
   it('escapes special characters in claim attributes without breaking the markup', () => {
@@ -135,10 +169,9 @@ describe('AnnotatedProfile', () => {
         ]}
       />
     )
-    const link = screen.getByRole('link')
-    expect(link.textContent).toBe('text is here')
-    expect(link).toHaveAttribute('aria-label', expect.stringContaining('Claim:'))
-    expect(link).toHaveAttribute('aria-label', expect.stringContaining('status Draft'))
+    const mark = screen.getByTestId('claim-highlight')
+    expect(mark.textContent).toBe('text is here')
+    expect(mark).toHaveAttribute('data-claim-status', ClaimStatusEnum.Draft)
   })
 
   it('rewrites relative image URLs against the owasp.org base', () => {

frontend/__tests__/unit/components/ClaimHighlight.test.tsx

@@ -1,24 +1,10 @@
 import { fireEvent, render, screen } from '@testing-library/react'
-import { useIsMobile } from 'hooks/useIsMobile'
 import { useRouter } from 'next/navigation'
 import React from 'react'
 
 import { ClaimStatusEnum } from 'types/__generated__/graphql'
 import ClaimHighlight from 'components/ClaimHighlight'
 
-jest.mock('hooks/useIsMobile', () => ({
-  useIsMobile: jest.fn(() => false),
-}))
-
-jest.mock('@heroui/tooltip', () => ({
-  Tooltip: ({ content, children }: { content: React.ReactNode; children: React.ReactNode }) => (
-    <>
-      <div data-testid="tooltip-content">{content}</div>
-      {children}
-    </>
-  ),
-}))
-
 jest.mock('@heroui/react', () => ({
   Popover: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
   PopoverTrigger: ({ children }: { children: React.ReactNode }) => <>{children}</>,
@@ -27,7 +13,21 @@ jest.mock('@heroui/react', () => ({
   ),
 }))
 
-const mockUseIsMobile = useIsMobile as jest.Mock
+jest.mock('@heroui/button', () => ({
+  Button: ({
+    children,
+    onPress,
+    className,
+  }: {
+    children: React.ReactNode
+    onPress?: () => void
+    className?: string
+  }) => (
+    <button type="button" onClick={onPress} className={className}>
+      {children}
+    </button>
+  ),
+}))
 
 const renderHighlight = (extra: Record<string, unknown> = {}) =>
   render(
@@ -46,7 +46,6 @@ const renderHighlight = (extra: Record<string, unknown> = {}) =>
 describe('ClaimHighlight', () => {
   beforeEach(() => {
     jest.clearAllMocks()
-    mockUseIsMobile.mockReturnValue(false)
   })
 
   it('renders a plain span when no claim key is present', () => {
@@ -59,49 +58,32 @@ describe('ClaimHighlight', () => {
     expect(span).not.toBeNull()
     expect(span?.className).toBe('foo')
     expect(span?.textContent).toBe('plain text')
-    expect(screen.queryByTestId('tooltip-content')).not.toBeInTheDocument()
+    expect(screen.queryByTestId('popover-content')).not.toBeInTheDocument()
   })
 
-  it('renders desktop tooltip with claim name and status badge', () => {
+  it('renders a popover with claim name, status badge, and View claim button', () => {
     renderHighlight()
-    const tooltip = screen.getByTestId('tooltip-content')
-    expect(tooltip).toHaveTextContent('My Claim')
-    expect(tooltip).toHaveTextContent('Approved')
-    expect(tooltip).toHaveTextContent('Click to view')
-    expect(screen.getByRole('link', { name: /Claim: My Claim, status Approved/i })).toBeVisible()
+    const popover = screen.getByTestId('popover-content')
+    expect(popover).toHaveTextContent('My Claim')
+    expect(popover).toHaveTextContent('Approved')
+    expect(screen.getByRole('button', { name: /View claim/i })).toBeInTheDocument()
   })
 
-  it('navigates on click when in desktop mode', () => {
+  it('navigates when the View claim button is clicked', () => {
     const push = (useRouter() as unknown as { push: jest.Mock }).push
     renderHighlight()
-    fireEvent.click(screen.getByRole('link'))
+    fireEvent.click(screen.getByRole('button', { name: /View claim/i }))
     expect(push).toHaveBeenCalledWith('/board/2025/candidates/alice/claims/my-claim')
   })
 
-  it('navigates on Enter key press', () => {
-    const push = (useRouter() as unknown as { push: jest.Mock }).push
-    renderHighlight()
-    fireEvent.keyDown(screen.getByRole('link'), { key: 'Enter' })
-    expect(push).toHaveBeenCalledWith('/board/2025/candidates/alice/claims/my-claim')
-  })
-
-  it('navigates on Space key press', () => {
-    const push = (useRouter() as unknown as { push: jest.Mock }).push
-    renderHighlight()
-    fireEvent.keyDown(screen.getByRole('link'), { key: ' ' })
-    expect(push).toHaveBeenCalledWith('/board/2025/candidates/alice/claims/my-claim')
+  it('falls back to Draft style when status is unknown', () => {
+    renderHighlight({ 'data-claim-status': 'MYSTERY' })
+    expect(screen.getByTestId('popover-content')).toHaveTextContent('Draft')
   })
 
-  it('ignores unrelated keys', () => {
-    const push = (useRouter() as unknown as { push: jest.Mock }).push
+  it('exposes the claim in the aria-label on the trigger', () => {
     renderHighlight()
-    fireEvent.keyDown(screen.getByRole('link'), { key: 'a' })
-    expect(push).not.toHaveBeenCalled()
-  })
-
-  it('falls back to Draft style when status is unknown', () => {
-    renderHighlight({ 'data-claim-status': 'MYSTERY' })
-    expect(screen.getByTestId('tooltip-content')).toHaveTextContent('Draft')
+    expect(screen.getByLabelText('Claim: My Claim, status Approved')).toBeInTheDocument()
   })
 
   it('falls back to unnamed in aria-label when name is missing', () => {
@@ -117,33 +99,4 @@ describe('ClaimHighlight', () => {
     )
     expect(screen.getByLabelText('Claim: unnamed, status Draft')).toBeInTheDocument()
   })
-
-  describe('mobile', () => {
-    beforeEach(() => {
-      mockUseIsMobile.mockReturnValue(true)
-    })
-
-    it('renders a popover with a View claim button', () => {
-      renderHighlight()
-      const popover = screen.getByTestId('popover-content')
-      expect(popover).toHaveTextContent('My Claim')
-      expect(popover).toHaveTextContent('Approved')
-      expect(screen.getByRole('button', { name: /View claim/i })).toBeInTheDocument()
-      expect(screen.queryByRole('link')).not.toBeInTheDocument()
-    })
-
-    it('navigates when the View claim button is clicked', () => {
-      const push = (useRouter() as unknown as { push: jest.Mock }).push
-      renderHighlight()
-      fireEvent.click(screen.getByRole('button', { name: /View claim/i }))
-      expect(push).toHaveBeenCalledWith('/board/2025/candidates/alice/claims/my-claim')
-    })
-
-    it('does not navigate on click of the highlight trigger', () => {
-      const push = (useRouter() as unknown as { push: jest.Mock }).push
-      renderHighlight()
-      fireEvent.click(screen.getByRole('button', { name: /Claim: My Claim, status Approved/i }))
-      expect(push).not.toHaveBeenCalled()
-    })
-  })
 })

frontend/__tests__/unit/hooks/useProfileSelection.test.tsx

@@ -9,14 +9,21 @@ type MockRangeInit = {
   rect?: Partial<DOMRect>
 }
 
-const setupSelection = ({ text, collapsed = false, rangeCount = 1, rect = {} }: MockRangeInit) => {
+const setupSelection = ({
+  text,
+  collapsed = false,
+  rangeCount = 1,
+  rect = {},
+  intersectsHighlight = false,
+}: MockRangeInit & { intersectsHighlight?: boolean }) => {
   const startContainer = document.createElement('span')
   const endContainer = document.createElement('span')
   const boundingRect = { top: 20, left: 10, width: 100, height: 16, ...rect } as DOMRect
   const range = {
     startContainer,
     endContainer,
     getBoundingClientRect: () => boundingRect,
+    intersectsNode: () => intersectsHighlight,
   }
   const selection = {
     rangeCount,
@@ -123,6 +130,29 @@ describe('useProfileSelection', () => {
     expect(result.current).toBeNull()
   })
 
+  it('returns null when the selection intersects an existing claim highlight', () => {
+    const container = document.createElement('div')
+    document.body.appendChild(container)
+    const highlight = document.createElement('span')
+    highlight.setAttribute('data-claim-highlight', 'true')
+    container.appendChild(highlight)
+    const { startContainer, endContainer } = setupSelection({
+      text: 'hello',
+      intersectsHighlight: true,
+    })
+    container.appendChild(startContainer)
+    container.appendChild(endContainer)
+
+    const containerRef = { current: container }
+    const { result } = renderHook(() => useProfileSelection(containerRef, true))
+
+    act(() => {
+      document.dispatchEvent(new Event('selectionchange'))
+    })
+
+    expect(result.current).toBeNull()
+  })
+
   it('removes listener on unmount', () => {
     const removeSpy = jest.spyOn(document, 'removeEventListener')
     const containerRef = createRef<HTMLDivElement>()

frontend/src/components/AnnotatedProfile.tsx

@@ -54,7 +54,7 @@ const resolveMediaSrc = <T,>(src: T, year: string): T | string => {
 const wrapClaims = (markdown: string, claims: ProfileClaim[]): string => {
   const eligible = claims
     .filter((c) => c.sourceText && !c.sourceText.includes('\n\n'))
-    .sort((a, b) => {
+    .toSorted((a, b) => {
       const lengthDiff = b.sourceText.length - a.sourceText.length
       if (lengthDiff !== 0) return lengthDiff
       return (STATUS_PRIORITY[b.status] ?? 0) - (STATUS_PRIORITY[a.status] ?? 0)
@@ -69,17 +69,29 @@ const wrapClaims = (markdown: string, claims: ProfileClaim[]): string => {
     ranges.push({ start, end, claim })
   }
 
-  return ranges
-    .sort((a, b) => b.start - a.start)
-    .reduce((acc, { start, end, claim }) => {
-      const open =
-        `<span data-claim-key="${escapeAttribute(claim.key)}"` +
-        ` data-claim-name="${escapeAttribute(claim.name)}"` +
-        ` data-claim-status="${claim.status}">`
-      return `${acc.slice(0, start)}${open}${acc.slice(start, end)}</span>${acc.slice(end)}`
-    }, markdown)
+  const orderedRanges = ranges.toSorted((a, b) => b.start - a.start)
+  return orderedRanges.reduce((acc, { start, end, claim }) => {
+    const open =
+      `<span data-claim-key="${escapeAttribute(claim.key)}"` +
+      ` data-claim-name="${escapeAttribute(claim.name)}"` +
+      ` data-claim-status="${claim.status}">`
+    return `${acc.slice(0, start)}${open}${acc.slice(start, end)}</span>${acc.slice(end)}`
+  }, markdown)
 }
 
+type MediaImgProps = ImgHTMLAttributes<HTMLImageElement> & { year: string }
+
+const MediaImg = ({ year, ...props }: MediaImgProps) => (
+  // eslint-disable-next-line @next/next/no-img-element, jsx-a11y/alt-text -- candidate markdown may reference arbitrary hosts and set alt itself
+  <img {...props} src={resolveMediaSrc(props.src, year)} />
+)
+
+type MediaSourceProps = SourceHTMLAttributes<HTMLSourceElement> & { year: string }
+
+const MediaSource = ({ year, ...props }: MediaSourceProps) => (
+  <source {...props} src={resolveMediaSrc(props.src, year)} />
+)
+
 const AnnotatedProfile = ({
   claims,
   isCandidate,
@@ -96,21 +108,9 @@ const AnnotatedProfile = ({
   const markdownOptions = useMemo(
     () => ({
       overrides: {
-        span: {
-          component: ClaimHighlight,
-          props: { year, login },
-        },
-        img: {
-          component: (props: ImgHTMLAttributes<HTMLImageElement>) => (
-            // eslint-disable-next-line @next/next/no-img-element, jsx-a11y/alt-text -- candidate markdown may reference arbitrary hosts and set alt itself
-            <img {...props} src={resolveMediaSrc(props.src, year)} />
-          ),
-        },
-        source: {
-          component: (props: SourceHTMLAttributes<HTMLSourceElement>) => (
-            <source {...props} src={resolveMediaSrc(props.src, year)} />
-          ),
-        },
+        span: { component: ClaimHighlight, props: { year, login } },
+        img: { component: MediaImg, props: { year } },
+        source: { component: MediaSource, props: { year } },
       },
     }),
     [year, login]