Skip to content

Commit c08611c

Browse files
divyanshu-vrSUPERGAMERDIVkasya
authored
Bug/fix scroll on Project Health Score click (#2050)
* fixed scroll position on project health badge * bug fixes * fixed sonaeCube and codeRabbit issues * CodeRabbit Suggestions Fixed * fixed CardDetailsPage * Update click event for MetricsScoreCircle * Fix tests * Fix checks and tests --------- Co-authored-by: Divyanshu Verma <[email protected]> Co-authored-by: Kate Golovanova <[email protected]>
1 parent 3018210 commit c08611c

File tree

8 files changed

+274
-34
lines changed

8 files changed

+274
-34
lines changed

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ describe('AnchorTitle Component', () => {
9898

9999
const titleElement = screen.getByText('Test')
100100
expect(titleElement).toHaveClass('flex', 'items-center', 'text-2xl', 'font-semibold')
101-
expect(titleElement).toHaveAttribute('id', 'anchor-title')
101+
expect(titleElement).toHaveAttribute('data-anchor-title', 'true')
102102
})
103103

104104
it('renders FontAwesome link icon', () => {
@@ -372,7 +372,7 @@ describe('AnchorTitle Component', () => {
372372
render(<AnchorTitle title="Heading Test" />)
373373

374374
const titleElement = screen.getByText('Heading Test')
375-
expect(titleElement).toHaveAttribute('id', 'anchor-title')
375+
expect(titleElement).toHaveAttribute('data-anchor-title', 'true')
376376
expect(titleElement).toHaveClass('text-2xl', 'font-semibold')
377377

378378
const container = document.getElementById('heading-test')
@@ -469,7 +469,7 @@ describe('AnchorTitle Component', () => {
469469
fireEvent.click(link)
470470

471471
expect(mockScrollTo).toHaveBeenCalledWith({
472-
top: 520,
472+
top: 20,
473473
behavior: 'smooth',
474474
})
475475

@@ -562,8 +562,8 @@ describe('AnchorTitle Component', () => {
562562
fireEvent.click(link)
563563
const secondCall = (mockScrollTo.mock.calls[1][0] as unknown as { top: number }).top
564564

565-
expect(firstCall).not.toBe(secondCall)
566-
expect(Math.abs(firstCall - secondCall)).toBe(30)
565+
expect(firstCall).toBe(secondCall)
566+
expect(Math.abs(firstCall - secondCall)).toBe(0)
567567

568568
mockScrollTo.mockRestore()
569569
mockGetElementById.mockRestore()

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,18 @@ jest.mock('components/LeadersList', () => ({
164164

165165
jest.mock('components/MetricsScoreCircle', () => ({
166166
__esModule: true,
167-
default: ({ score, ...props }: { score: number; [key: string]: unknown }) => (
168-
<div data-testid="metrics-score-circle" {...props}>
167+
default: ({
168+
score,
169+
clickable,
170+
onClick: _onClick,
171+
...props
172+
}: {
173+
score: number
174+
clickable?: boolean
175+
onClick?: () => void
176+
[key: string]: unknown
177+
}) => (
178+
<div data-testid="metrics-score-circle" role={clickable ? 'button' : undefined} {...props}>
169179
Score: {score}
170180
</div>
171181
),
@@ -724,7 +734,7 @@ describe('CardDetailsPage', () => {
724734
})
725735

726736
describe('Event Handling', () => {
727-
it('renders clickable health metrics link', () => {
737+
it('renders clickable health metrics button', () => {
728738
render(
729739
<CardDetailsPage
730740
{...defaultProps}
@@ -733,8 +743,9 @@ describe('CardDetailsPage', () => {
733743
/>
734744
)
735745

736-
const healthLink = screen.getByRole('link')
737-
expect(healthLink).toHaveAttribute('href', '#issues-trend')
746+
const healthButton = screen.getByRole('button')
747+
expect(healthButton).toBeInTheDocument()
748+
expect(screen.getByTestId('metrics-score-circle')).toBeInTheDocument()
738749
})
739750

740751
it('renders social links with correct hrefs and target attributes', () => {

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

Lines changed: 167 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { render, screen } from '@testing-library/react'
1+
import { render, screen, fireEvent } from '@testing-library/react'
22
import React from 'react'
33
import '@testing-library/jest-dom'
44
import MetricsScoreCircle from 'components/MetricsScoreCircle'
@@ -172,14 +172,22 @@ describe('MetricsScoreCircle', () => {
172172
})
173173

174174
// Test 9: Event handling - hover effects (visual testing through classes)
175-
it('has hover effect classes applied', () => {
176-
const { container } = render(<MetricsScoreCircle score={75} />)
175+
it('has hover effect classes applied when clickable', () => {
176+
const { container } = render(<MetricsScoreCircle score={75} clickable={true} />)
177177

178178
// Check for hover-related classes
179179
const hoverElement = container.querySelector('[class*="hover:"]')
180180
expect(hoverElement).toBeInTheDocument()
181181
})
182182

183+
it('does not have hover effect classes when not clickable', () => {
184+
const { container } = render(<MetricsScoreCircle score={75} clickable={false} />)
185+
186+
// Should not have hover-related classes
187+
const hoverElement = container.querySelector('[class*="hover:"]')
188+
expect(hoverElement).not.toBeInTheDocument()
189+
})
190+
183191
// Test 10: Component integration test
184192
it('integrates all features correctly for a low score', () => {
185193
const { container } = render(<MetricsScoreCircle score={15} />)
@@ -218,4 +226,160 @@ describe('MetricsScoreCircle', () => {
218226
'Current Project Health Score'
219227
)
220228
})
229+
230+
// Test 11: Click handling functionality
231+
describe('click handling', () => {
232+
it('calls onClick when clickable and onClick provided', () => {
233+
const mockOnClick = jest.fn()
234+
render(<MetricsScoreCircle score={75} clickable={true} onClick={mockOnClick} />)
235+
236+
const circleElement = screen.getByText('75').closest('.group')
237+
if (circleElement) {
238+
fireEvent.click(circleElement)
239+
}
240+
241+
expect(mockOnClick).toHaveBeenCalledTimes(1)
242+
})
243+
244+
it('does not call onClick when not clickable', () => {
245+
const mockOnClick = jest.fn()
246+
render(<MetricsScoreCircle score={75} clickable={false} onClick={mockOnClick} />)
247+
248+
const circleElement = screen.getByText('75').closest('.group')
249+
if (circleElement) {
250+
fireEvent.click(circleElement)
251+
}
252+
253+
expect(mockOnClick).not.toHaveBeenCalled()
254+
})
255+
256+
it('does not call onClick when no onClick provided', () => {
257+
render(<MetricsScoreCircle score={75} clickable={true} />)
258+
259+
const circleElement = screen.getByText('75').closest('.group')
260+
if (circleElement) {
261+
fireEvent.click(circleElement)
262+
}
263+
// Should not throw any errors - test passes if no exception is thrown
264+
expect(circleElement).toBeInTheDocument()
265+
})
266+
267+
it('has cursor pointer when clickable', () => {
268+
const { container } = render(<MetricsScoreCircle score={75} clickable={true} />)
269+
270+
const clickableElement = container.querySelector('[class*="cursor-pointer"]')
271+
expect(clickableElement).toBeInTheDocument()
272+
})
273+
274+
it('does not have cursor pointer when not clickable', () => {
275+
const { container } = render(<MetricsScoreCircle score={75} clickable={false} />)
276+
277+
const clickableElement = container.querySelector('[class*="cursor-pointer"]')
278+
expect(clickableElement).not.toBeInTheDocument()
279+
})
280+
})
281+
282+
// Test 12: Accessibility for clickable component
283+
describe('accessibility for clickable component', () => {
284+
it('has button role when clickable', () => {
285+
render(<MetricsScoreCircle score={75} clickable={true} />)
286+
287+
const buttonElement = screen.getByRole('button')
288+
expect(buttonElement).toBeInTheDocument()
289+
})
290+
291+
it('does not have button role when not clickable', () => {
292+
render(<MetricsScoreCircle score={75} clickable={false} />)
293+
294+
const buttonElement = screen.queryByRole('button')
295+
expect(buttonElement).not.toBeInTheDocument()
296+
})
297+
298+
it('has tabIndex when clickable', () => {
299+
const { container } = render(<MetricsScoreCircle score={75} clickable={true} />)
300+
301+
const clickableElement = container.querySelector('[tabindex="0"]')
302+
expect(clickableElement).toBeInTheDocument()
303+
})
304+
305+
it('does not have tabIndex when not clickable', () => {
306+
const { container } = render(<MetricsScoreCircle score={75} clickable={false} />)
307+
308+
const clickableElement = container.querySelector('[tabindex]')
309+
expect(clickableElement).not.toBeInTheDocument()
310+
})
311+
312+
it('handles keyboard navigation when clickable', () => {
313+
const mockOnClick = jest.fn()
314+
render(<MetricsScoreCircle score={75} clickable={true} onClick={mockOnClick} />)
315+
316+
const buttonElement = screen.getByRole('button')
317+
318+
// Test Enter key
319+
fireEvent.keyDown(buttonElement, { key: 'Enter' })
320+
expect(mockOnClick).toHaveBeenCalledTimes(1)
321+
322+
// Test Space key
323+
fireEvent.keyDown(buttonElement, { key: ' ' })
324+
expect(mockOnClick).toHaveBeenCalledTimes(2)
325+
})
326+
327+
it('does not handle keyboard navigation when not clickable', () => {
328+
const mockOnClick = jest.fn()
329+
render(<MetricsScoreCircle score={75} clickable={false} onClick={mockOnClick} />)
330+
331+
const circleElement = screen.getByText('75').closest('.group')
332+
if (circleElement) {
333+
fireEvent.keyDown(circleElement, { key: 'Enter' })
334+
fireEvent.keyDown(circleElement, { key: ' ' })
335+
}
336+
337+
expect(mockOnClick).not.toHaveBeenCalled()
338+
})
339+
})
340+
341+
// Test 13: Maintains existing functionality with new props
342+
it('maintains all existing functionality when clickable is true', () => {
343+
const { container } = render(<MetricsScoreCircle score={25} clickable={true} />)
344+
345+
// Should still have red styling
346+
expect(container.querySelector('[class*="bg-red"]')).toBeInTheDocument()
347+
348+
// Should still have pulse animation
349+
expect(container.querySelector('[class*="animate-pulse"]')).toBeInTheDocument()
350+
351+
// Should still display correct score
352+
expect(screen.getByText('25')).toBeInTheDocument()
353+
354+
// Should still have tooltip
355+
expect(screen.getByTestId('tooltip-wrapper')).toHaveAttribute(
356+
'data-content',
357+
'Current Project Health Score'
358+
)
359+
360+
// Should have hover effects
361+
expect(container.querySelector('[class*="hover:"]')).toBeInTheDocument()
362+
})
363+
364+
it('maintains all existing functionality when clickable is false', () => {
365+
const { container } = render(<MetricsScoreCircle score={25} clickable={false} />)
366+
367+
// Should still have red styling
368+
expect(container.querySelector('[class*="bg-red"]')).toBeInTheDocument()
369+
370+
// Should still have pulse animation
371+
expect(container.querySelector('[class*="animate-pulse"]')).toBeInTheDocument()
372+
373+
// Should still display correct score
374+
expect(screen.getByText('25')).toBeInTheDocument()
375+
376+
// Should still have tooltip
377+
expect(screen.getByTestId('tooltip-wrapper')).toHaveAttribute(
378+
'data-content',
379+
'Current Project Health Score'
380+
)
381+
382+
// Should NOT have hover effects
383+
expect(container.querySelector('[class*="hover:"]')).not.toBeInTheDocument()
384+
})
221385
})

frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ const ProjectHealthMetricsDetails: FC = () => {
7272
/>
7373
</div>
7474
<div className="flex items-center gap-2">
75-
<MetricsScoreCircle score={metricsLatest.score} />
75+
<MetricsScoreCircle score={metricsLatest.score} clickable={false} />
7676
<GeneralCompliantComponent
7777
title={
7878
metricsLatest.isFundingRequirementsCompliant

frontend/src/components/AnchorTitle.tsx

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { faLink } from '@fortawesome/free-solid-svg-icons'
22
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'
33
import React, { useEffect, useCallback } from 'react'
4+
import { scrollToAnchor, scrollToAnchorWithHistory } from 'utils/scrollToAnchor'
45
import slugify from 'utils/slugify'
56

67
interface AnchorTitleProps {
@@ -13,20 +14,12 @@ const AnchorTitle: React.FC<AnchorTitleProps> = ({ title }) => {
1314
const href = `#${id}`
1415

1516
const scrollToElement = useCallback(() => {
16-
const element = document.getElementById(id)
17-
if (element) {
18-
const headingHeight =
19-
(element.querySelector('div#anchor-title') as HTMLElement)?.offsetHeight || 0
20-
const yOffset = -headingHeight - 50
21-
const y = element.getBoundingClientRect().top + window.pageYOffset + yOffset
22-
window.scrollTo({ top: y, behavior: 'smooth' })
23-
}
17+
scrollToAnchor(id)
2418
}, [id])
2519

2620
const handleClick = (event: React.MouseEvent<HTMLAnchorElement, MouseEvent>) => {
2721
event.preventDefault()
28-
scrollToElement()
29-
window.history.pushState(null, '', href)
22+
scrollToAnchorWithHistory(id)
3023
}
3124

3225
useEffect(() => {
@@ -50,7 +43,7 @@ const AnchorTitle: React.FC<AnchorTitleProps> = ({ title }) => {
5043
return (
5144
<div id={id} className="relative">
5245
<div className="group relative flex items-center">
53-
<div className="flex items-center text-2xl font-semibold" id="anchor-title">
46+
<div className="flex items-center text-2xl font-semibold" data-anchor-title="true">
5447
{title}
5548
</div>
5649
<a

frontend/src/components/CardDetailsPage.tsx

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@ import {
1010
} from '@fortawesome/free-solid-svg-icons'
1111
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'
1212
import upperFirst from 'lodash/upperFirst'
13-
import Link from 'next/link'
1413
import { useRouter } from 'next/navigation'
1514
import { useSession } from 'next-auth/react'
1615
import type { ExtendedSession } from 'types/auth'
1716
import type { DetailsCardProps } from 'types/card'
1817
import { IS_PROJECT_HEALTH_ENABLED } from 'utils/credentials'
18+
import { scrollToAnchor } from 'utils/scrollToAnchor'
1919
import { getSocialIcon } from 'utils/urlIconMappings'
2020
import AnchorTitle from 'components/AnchorTitle'
2121
import ChapterMapWrapper from 'components/ChapterMapWrapper'
@@ -96,9 +96,11 @@ const DetailsCard = ({
9696
</button>
9797
)}
9898
{IS_PROJECT_HEALTH_ENABLED && type === 'project' && healthMetricsData.length > 0 && (
99-
<Link href="#issues-trend">
100-
<MetricsScoreCircle score={healthMetricsData[0].score} />
101-
</Link>
99+
<MetricsScoreCircle
100+
score={healthMetricsData[0].score}
101+
clickable={true}
102+
onClick={() => scrollToAnchor('issues-trend')}
103+
/>
102104
)}
103105
</div>
104106
{!isActive && (

0 commit comments

Comments
 (0)