Bug/fix scroll on Project Health Score click #2050
Conversation
|
Warning Rate limit exceeded@kasya has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 37 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughRefactors in-page scrolling into utilities (scrollToAnchor / scrollToAnchorWithHistory), updates AnchorTitle to use data-anchor-title, replaces Link with a clickable MetricsScoreCircle wired to scrollToAnchor, and updates related unit tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/__tests__/unit/components/AnchorTitle.test.tsx(4 hunks)frontend/__tests__/unit/components/MetricsScoreCircle.test.tsx(2 hunks)frontend/src/components/AnchorTitle.tsx(2 hunks)frontend/src/components/CardDetailsPage.tsx(2 hunks)frontend/src/components/MetricsScoreCircle.tsx(1 hunks)frontend/src/utils/scrollToAnchor.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-20T16:12:59.256Z
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Applied to files:
frontend/src/components/CardDetailsPage.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Applied to files:
frontend/src/components/CardDetailsPage.tsx
🧬 Code Graph Analysis (2)
frontend/src/components/CardDetailsPage.tsx (1)
frontend/src/utils/scrollToAnchor.ts (1)
scrollToAnchor(1-24)
frontend/src/components/AnchorTitle.tsx (1)
frontend/src/utils/scrollToAnchor.ts (2)
scrollToAnchor(1-24)scrollToAnchorWithHistory(26-37)
🔇 Additional comments (10)
frontend/src/components/MetricsScoreCircle.tsx (2)
4-7: LGTM: Well-designed interface with backward compatibility.The new
MetricsScoreCirclePropsinterface properly adds the optionalonClickprop while maintaining backward compatibility. The interface is clean and follows TypeScript best practices.
17-21: LGTM: Proper null-safe click handler implementation.The
handleClickfunction correctly checks for the existence ofonClickbefore calling it, preventing any potential runtime errors. The implementation is clean and follows defensive programming practices.frontend/__tests__/unit/components/MetricsScoreCircle.test.tsx (2)
1-1: LGTM: Proper import of fireEvent for interaction testing.The addition of
fireEventimport is necessary for testing the new click functionality and is correctly imported from the testing library.
222-285: Excellent test coverage for click functionality.The new test suite comprehensively covers all aspects of the click handling feature:
- Verifies onClick callback is invoked when provided
- Ensures no errors occur when onClick is not provided
- Tests conditional cursor styling application
- Confirms existing functionality remains intact when onClick is present
The tests are well-structured, use appropriate selectors, and follow testing best practices.
frontend/src/utils/scrollToAnchor.ts (1)
26-37: LGTM: Clean history management with proper error handling.The
scrollToAnchorWithHistoryfunction has a clean design with:
- Clear separation of concerns (scrolling vs. history management)
- Optional history updates via the
updateHistoryparameter- Proper error handling for history operations
The default parameter value and error handling approach are appropriate here.
frontend/src/components/CardDetailsPage.tsx (1)
15-15: LGTM: Proper import of scroll utility.The import of
scrollToAnchorfrom the new utility module is correct and follows the established import patterns in the file.frontend/src/components/AnchorTitle.tsx (2)
4-4: LGTM: Clean import of scroll utilities.The import of both
scrollToAnchorandscrollToAnchorWithHistoryfrom the new utility module is correct and enables the consolidation of scroll logic.
16-23: Excellent refactoring to use centralized scroll utilities.The refactoring successfully consolidates scroll logic by:
scrollToElementnow simply callsscrollToAnchor(id)handleClicknow usesscrollToAnchorWithHistory(id)This eliminates code duplication, follows DRY principles, and ensures consistent scroll behavior across the application. The public API remains unchanged, maintaining backward compatibility.
frontend/__tests__/unit/components/AnchorTitle.test.tsx (2)
261-267: Test updates are consistent with new scroll behavior.The test expectations have been consistently updated across multiple test scenarios to expect
top: -10, indicating that the AnchorTitle component now uses the new scrollToAnchor utility. The updates cover:
- Click behavior tests
- useEffect-based scrolling tests
- pageYOffset handling tests
Assuming the offset calculation is correct (pending verification above), these test updates properly reflect the new behavior.
Also applies to: 472-475
203-226: No changes needed to scroll offset calculation. The tests mock an element top of 100 and a heading height of 30, so the utility computes yOffset = –30 – 80 = –110 and then y = 100 + (–110) = –10. Theexpect(...top: -10)assertions correctly reflect this.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
frontend/src/components/CardDetailsPage.tsx (2)
65-75: Preserve URL hash/back navigation by using the history-aware scroll helper.
You prevent the default anchor behavior, so the URL hash doesn’t update. If you want standard anchor semantics (hash update + proper back/forward behavior), call the history-aware variant.Proposed diff:
- import { scrollToAnchor } from 'utils/scrollToAnchor' + import { scrollToAnchorWithHistory } from 'utils/scrollToAnchor' ... - onClick={(e) => { + onClick={(e) => { e.preventDefault() - scrollToAnchor('issues-trend') + scrollToAnchorWithHistory('issues-trend') }}If hash updates are intentionally avoided here, please confirm and we’ll skip this change.
65-75: Small a11y win: add an accessible label and support Space key.
Anchor already supports Enter; adding aria-label clarifies purpose, and handling Space aligns with the future plan mentioned in the PR.Proposed diff:
- <a + <a href="#issues-trend" + aria-label="View Issues trend" onClick={(e) => { e.preventDefault() - scrollToAnchor('issues-trend') + scrollToAnchorWithHistory('issues-trend') }} + onKeyDown={(e) => { + if (e.key === ' ' || e.key === 'Spacebar') { + e.preventDefault() + scrollToAnchorWithHistory('issues-trend') + } + }} >frontend/src/utils/scrollToAnchor.ts (2)
1-21: Solid offset-aware scrolling; add SSR guard and respect reduced-motion.
Function works as intended. Two robustness tweaks:
- Guard for SSR to avoid touching window/document in non-browser contexts.
- Respect prefers-reduced-motion to avoid forced smooth scrolling for users who opt out.
Proposed diff:
export function scrollToAnchor(targetId: string, additionalOffset = 80): void { try { + if (typeof window === 'undefined' || typeof document === 'undefined') { + return + } const element = document.getElementById(targetId) if (!element) { - return + // Optional: dev-only warning to aid debugging + if (typeof process !== 'undefined' && process.env && process.env.NODE_ENV !== 'production') { + // eslint-disable-next-line no-console + console.warn(`scrollToAnchor: element with id "${targetId}" not found`) + } + return } - const anchorElement = element.querySelector('div#anchor-title') + const anchorElement = element.querySelector('div#anchor-title') const headingHeight = (anchorElement instanceof HTMLElement) ? anchorElement.offsetHeight : 0 const yOffset = -headingHeight - additionalOffset // Use modern window.scrollY instead of deprecated pageYOffset const y = element.getBoundingClientRect().top + window.scrollY + yOffset - window.scrollTo({ - top: y, - behavior: 'smooth' - }) + const prefersReducedMotion = + typeof window.matchMedia === 'function' && + window.matchMedia('(prefers-reduced-motion: reduce)').matches + window.scrollTo({ + top: y, + behavior: prefersReducedMotion ? 'auto' : 'smooth' + }) } catch { // Silently handle scroll errors } }
23-34: Avoid redundant history entries; push only when the hash changes.
Prevents cluttering the history stack with identical hashes and keeps back navigation cleaner.Proposed diff:
export const scrollToAnchorWithHistory = (targetId: string, updateHistory = true): void => { scrollToAnchor(targetId) if (updateHistory) { try { - const href = `#${targetId}` - window.history.pushState(null, '', href) + if (typeof window === 'undefined') return + const href = `#${targetId}` + if (window.location.hash !== href) { + window.history.pushState(null, '', href) + } } catch { // Silently handle history update errors } } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/__tests__/unit/components/AnchorTitle.test.tsx(2 hunks)frontend/__tests__/unit/components/MetricsScoreCircle.test.tsx(2 hunks)frontend/src/components/CardDetailsPage.tsx(3 hunks)frontend/src/utils/scrollToAnchor.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/tests/unit/components/MetricsScoreCircle.test.tsx
- frontend/tests/unit/components/AnchorTitle.test.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-20T16:12:59.256Z
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Applied to files:
frontend/src/components/CardDetailsPage.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Applied to files:
frontend/src/components/CardDetailsPage.tsx
🧬 Code Graph Analysis (1)
frontend/src/components/CardDetailsPage.tsx (1)
frontend/src/utils/scrollToAnchor.ts (1)
scrollToAnchor(1-21)
🔇 Additional comments (2)
frontend/src/components/CardDetailsPage.tsx (2)
15-15: Import is correct and consistent with new utility.
No issues. Named import matches the new module.
261-261: No functional change.
Likely formatting/whitespace-only.
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
@divyanshu-vr Hi! There are some conflicts with your current changes. Please resolve those so I could review the PR 👍🏼
Hi @kasya, I've resolved the conflicts. You can review the PR now 👍 |
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
This works! I did left some comments about implementation though and also you need to update your PR - you removed a bunch of stuff that we still use in the CardDetailsPage component.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/src/components/MetricsScoreCircle.tsx (1)
31-49: Consider using a semantic button for full a11y and native keyboard/focus behaviors.You’ve added role="button", tabIndex, and keyboard handling—good step. Using a real would give you native keyboard support, focus management, and better semantics without custom handlers.
If you choose to swap:
- Keep the same classes.
- Add type="button".
- Use disabled={!clickable} instead of role/tabIndex/onKeyDown.
Happy to provide a targeted diff if you want to proceed.
🧹 Nitpick comments (8)
frontend/src/components/MetricsScoreCircle.tsx (3)
4-8: Unify event typing for onClick; remove unsafe cast in keyboard handler.Right now onKeyDown passes a KeyboardEvent into onClick by casting to MouseEvent. This is type-unsafe and may break consumers that rely on MouseEvent fields. Prefer a union type for the callback and remove the cast. Also, avoid importing the bare MouseEvent type to keep the React-prefixed event types explicit.
Apply the following diff:
-import React, { FC, MouseEvent } from 'react' +import React, { FC } from 'react' interface MetricsScoreCircleProps { score: number - onClick?: (e: MouseEvent<HTMLDivElement>) => void + onClick?: (e: React.MouseEvent<HTMLDivElement> | React.KeyboardEvent<HTMLDivElement>) => void clickable?: boolean } @@ - const handleClick = (e: MouseEvent<HTMLDivElement>) => { + const handleClick = (e: React.MouseEvent<HTMLDivElement>) => { if (clickable && onClick) { onClick(e) } } @@ - ? (e: React.KeyboardEvent<HTMLDivElement>) => { + ? (e: React.KeyboardEvent<HTMLDivElement>) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault() - // For keyboard navigation, call onClick directly since we don't need the event object - if (onClick) { - onClick(e as unknown as React.MouseEvent<HTMLDivElement>) - } + if (onClick) onClick(e) } } : undefined }Also applies to: 18-22, 36-48
33-33: Avoid attaching onClick when not interactive.Minor: The handler is attached even when not clickable. Reduce unnecessary listeners and align semantics by only attaching it when clickable.
- onClick={handleClick} + onClick={clickable ? handleClick : undefined}
24-28: Add a visible focus style for keyboard users.When clickable, there’s no visible focus outline. Add a focus-visible ring/outline to improve keyboard navigation clarity.
Example (adjust colors to your design system):
- Add something like: focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-blue-500 to the clickableClasses.
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx (1)
75-75: Explicit clickable={false} is redundant (defaults to false).Leaving it is fine for clarity, but you can drop the prop to reduce noise since clickable defaults to false.
- <MetricsScoreCircle score={metricsLatest.score} clickable={false} /> + <MetricsScoreCircle score={metricsLatest.score} />frontend/__tests__/unit/components/MetricsScoreCircle.test.tsx (4)
175-181: Test assertion is fine, but a bit brittle.Querying for [class*="hover:"] is vulnerable to unrelated hover utility classes. It’s acceptable here, just be aware this may require updates if class names refactor.
Would you like me to tighten this to assert for either cursor-pointer presence or the overlay’s group-hover class to reduce false positives?
232-242: Prefer getByRole('button') for clickable scenario.You can make the test more robust by targeting the accessible role rather than relying on the group class and DOM traversal.
- const circleElement = screen.getByText('75').closest('.group') - if (circleElement) { - fireEvent.click(circleElement) - } - expect(mockOnClick).toHaveBeenCalledTimes(1) + const buttonElement = screen.getByRole('button') + fireEvent.click(buttonElement) + expect(mockOnClick).toHaveBeenCalledTimes(1)
244-254: Strengthen negative click test by clicking the outer circle element.Currently the test may pass vacuously if no element is found. Target the outer circle inside the tooltip wrapper and assert the handler is not called.
- const circleElement = screen.getByText('75').closest('.group') - if (circleElement) { - fireEvent.click(circleElement) - } - expect(mockOnClick).not.toHaveBeenCalled() + const wrapper = screen.getByTestId('tooltip-wrapper') + const outerCircle = wrapper.querySelector('[class*="rounded-full"]') as HTMLElement + fireEvent.click(outerCircle) + expect(mockOnClick).not.toHaveBeenCalled()
327-339: Avoid vacuous pass; dispatch key events on the outer circle.Like the click test, this can silently skip sending events. Target the outer circle so we know the events are actually fired and ignored.
- const circleElement = screen.getByText('75').closest('.group') - if (circleElement) { - fireEvent.keyDown(circleElement, { key: 'Enter' }) - fireEvent.keyDown(circleElement, { key: ' ' }) - } - expect(mockOnClick).not.toHaveBeenCalled() + const wrapper = screen.getByTestId('tooltip-wrapper') + const outerCircle = wrapper.querySelector('[class*="rounded-full"]') as HTMLElement + fireEvent.keyDown(outerCircle, { key: 'Enter' }) + fireEvent.keyDown(outerCircle, { key: ' ' }) + expect(mockOnClick).not.toHaveBeenCalled()
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
frontend/__tests__/unit/components/MetricsScoreCircle.test.tsx(3 hunks)frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx(1 hunks)frontend/src/components/CardDetailsPage.tsx(2 hunks)frontend/src/components/MetricsScoreCircle.tsx(2 hunks)frontend/src/utils/scrollToAnchor.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/utils/scrollToAnchor.ts
- frontend/src/components/CardDetailsPage.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-20T16:12:59.256Z
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Applied to files:
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Applied to files:
frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/__tests__/unit/components/CardDetailsPage.test.tsx (2)
158-163: Widen LeadersList mock prop type to avoid future mismatchThe real component may accept an array of leaders; typing the mock as leaders: string could cause TS friction down the line. Make it looser to keep tests resilient.
-jest.mock('components/LeadersList', () => ({ +jest.mock('components/LeadersList', () => ({ __esModule: true, - default: ({ leaders, ...props }: { leaders: string;[key: string]: unknown }) => ( + default: ({ leaders, ...props }: { leaders: unknown; [key: string]: unknown }) => ( <span data-testid="leaders-list" {...props}> {leaders} </span> ), }))
731-743: Tighten the “button” assertion to the target elementgetByRole('button') may become brittle if more buttons are added. Assert the role on the specific metrics element to avoid false positives.
- const healthButton = screen.getByRole('button') - expect(healthButton).toBeInTheDocument() - expect(screen.getByTestId('metrics-score-circle')).toBeInTheDocument() + const circle = screen.getByTestId('metrics-score-circle') + expect(circle).toBeInTheDocument() + expect(circle).toHaveAttribute('role', 'button')
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
frontend/__tests__/unit/components/CardDetailsPage.test.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-20T16:12:59.256Z
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Applied to files:
frontend/__tests__/unit/components/CardDetailsPage.test.tsx
📚 Learning: 2025-06-20T16:12:59.256Z
Learnt from: ahmedxgouda
PR: OWASP/Nest#1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a safety check that ensures HealthMetrics component is only rendered when healthMetricsData exists and has at least one element: `healthMetricsData && healthMetricsData.length > 0`. This makes accessing data[0] safe within the HealthMetrics component.
Applied to files:
frontend/__tests__/unit/components/CardDetailsPage.test.tsx
🪛 GitHub Actions: Run CI/CD
frontend/__tests__/unit/components/CardDetailsPage.test.tsx
[error] 170-170: ESLint: 'onClick' is defined but never used. Allowed unused args must match /^_/ (no-unused-vars). Command 'eslint . --config eslint.config.mjs --max-warnings=0' failed (exit code 1) during 'pnpm run lint:check'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CodeQL (javascript-typescript)
🔇 Additional comments (1)
frontend/__tests__/unit/components/CardDetailsPage.test.tsx (1)
127-132: HealthMetrics mock signature looks goodAccepts data and forwards props. Safe to call data.length in this test context.
|
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
@divyanshu-vr Thanks for working on this!
I refactored MetricsScoreCircle to handle click logic internally instead of wrapping with Link components. Also added onClick and clickable props for flexible usage, improved accessibility with proper button roles and keyboard navigation. This provides cleaner architecture and better separation of concerns.
These changes make it even better thank you, @kasya ! I had implemented keyboard navigation initially but later removed it, as I was asked on the issue page to leave it for a future enhancement. Do you have any specific feedback for me? I’m new to OSS and still learning best practices. |
3 participants
Proposed change
Fixed scroll on Project Health Score click
Resolves #2034
Fixed the bug and added tests.
Feature- we can add a feature where pressing Enter or Spacebar on the badge scrolls down. I've optimized it for so, if we ever need to do it in the future.
Checklist
make check-testlocally; all checks and tests passed.