fix(a11y): Correct color contrast for WCAG AA compliance (#3797)#3798
fix(a11y): Correct color contrast for WCAG AA compliance (#3797)#3798Srizdebnath wants to merge 3 commits intoOWASP:mainfrom
Conversation
Summary by CodeRabbit
WalkthroughThis PR updates Tailwind color utilities across many frontend pages, components, and tests to improve contrast (e.g., Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
11 issues found across 66 files
Confidence score: 3/5
- Multiple link color changes to
text-blue-300reduce contrast on light backgrounds and fail WCAG AA, which is a concrete accessibility regression for users (e.g.,frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx,frontend/src/app/committees/[committeeKey]/page.tsx). - These contrast drops are repeated across shared components like
frontend/src/components/Release.tsx,frontend/src/components/ModuleCard.tsx, andfrontend/src/components/ContributorsList.tsx, so the impact is broad and user-facing despite being straightforward to fix. - Pay close attention to
frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx,frontend/src/app/committees/[committeeKey]/page.tsx,frontend/src/components/Release.tsx,frontend/src/components/ModuleCard.tsx,frontend/src/components/ContributorsList.tsx,frontend/src/components/CardDetailsPage.tsx,frontend/src/app/chapters/[chapterKey]/page.tsx,frontend/src/app/board/[year]/candidates/page.tsx- link/icon contrast regressions from lighter blue text.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="frontend/src/app/chapters/[chapterKey]/page.tsx">
<violation number="1" location="frontend/src/app/chapters/[chapterKey]/page.tsx:64">
P2: This change makes the link text lighter (blue-300), which reduces contrast and can worsen WCAG AA compliance on light backgrounds. Consider using a darker shade (e.g., blue-600/700) to improve contrast.</violation>
</file>
<file name="frontend/src/app/board/[year]/candidates/page.tsx">
<violation number="1" location="frontend/src/app/board/[year]/candidates/page.tsx:353">
P2: `text-blue-300` on the white card background has insufficient contrast (≈1.7:1), so this change fails WCAG AA and reduces contrast relative to the previous color. Use a darker blue for light mode and keep a lighter shade in dark mode.</violation>
</file>
<file name="frontend/src/components/ModuleCard.tsx">
<violation number="1" location="frontend/src/components/ModuleCard.tsx:49">
P2: The new text color is lighter (text-blue-300), which lowers contrast against the light background. This runs counter to the WCAG AA contrast goal; use a darker blue shade instead to preserve contrast.</violation>
</file>
<file name="frontend/src/components/Release.tsx">
<violation number="1" location="frontend/src/components/Release.tsx:53">
P2: This change makes the link text lighter (blue-300), which reduces contrast against the light background and undermines the WCAG AA contrast fix. Use a darker blue shade here.</violation>
<violation number="2" location="frontend/src/components/Release.tsx:72">
P2: This change lightens the link text (blue-300), reducing contrast on the light background. Use a darker blue shade to meet WCAG AA contrast.</violation>
</file>
<file name="frontend/src/components/ContributorsList.tsx">
<violation number="1" location="frontend/src/components/ContributorsList.tsx:63">
P2: The new `text-blue-300` is lighter than the previous `text-blue-400`, and on the unchanged `bg-gray-200` background it reduces contrast (≈1.46:1 vs ≈2.05:1). This makes the link less legible and moves contrast in the wrong direction. Use a darker shade for the link text on this light background (e.g., blue-700).</violation>
</file>
<file name="frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx">
<violation number="1" location="frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx:72">
P2: Switching this link to `text-blue-300` reduces contrast on the white background and is still below the 4.5:1 WCAG AA requirement for normal text. This is likely a regression from the accessibility fixes; use a darker blue that meets AA (e.g., blue-600).</violation>
</file>
<file name="frontend/src/app/committees/[committeeKey]/page.tsx">
<violation number="1" location="frontend/src/app/committees/[committeeKey]/page.tsx:65">
P2: The updated link color uses a lighter shade (text-blue-300) on a white background, dropping contrast to ~1.8:1 and failing WCAG AA. Use a darker shade (e.g., text-blue-600) so the URL remains accessible on light backgrounds.</violation>
</file>
<file name="frontend/src/components/CardDetailsPage.tsx">
<violation number="1" location="frontend/src/components/CardDetailsPage.tsx:426">
P2: `text-blue-300` is too light on the light-theme milestone card (`bg-gray-200`) and fails WCAG AA contrast. Use a darker light-theme color and keep a light dark-mode override instead of a single light shade.</violation>
<violation number="2" location="frontend/src/components/CardDetailsPage.tsx:447">
P2: The milestone title link color `text-blue-300` fails WCAG AA on the light gray card background. Use a darker light-theme shade with a dark-mode override.</violation>
<violation number="3" location="frontend/src/components/CardDetailsPage.tsx:526">
P2: Social link icons use `text-blue-300` on the light theme’s white background, which fails WCAG AA contrast. Use a darker light-theme color with a dark-mode override.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
frontend/src/components/MultiSearch.tsx (1)
287-296:⚠️ Potential issue | 🟡 MinorDark-mode hover on Algolia link is now a no-op.
dark:text-gray-200anddark:hover:text-gray-200resolve to the same color, so hovering provides no visual feedback in dark mode. The original likely haddark:text-gray-400 → dark:hover:text-gray-200which gave a visible lightening effect. Now that the base was bumped togray-200for contrast, the hover lost its differentiation.Consider changing the hover to
dark:hover:text-whiteto restore the feedback while keeping the improved base contrast.Proposed fix
- className="flex items-center justify-center gap-2 bg-white py-2 text-gray-500 hover:text-gray-700 dark:bg-gray-800 dark:text-gray-200 dark:hover:text-gray-200" + className="flex items-center justify-center gap-2 bg-white py-2 text-gray-500 hover:text-gray-700 dark:bg-gray-800 dark:text-gray-200 dark:hover:text-white"frontend/src/components/BreadCrumbs.tsx (1)
41-49:⚠️ Potential issue | 🟡 MinorMissing
dark:hovertext color — hover contrast may fail AA in dark mode.
hover:text-blue-700applies in both light and dark modes. In dark mode, blue-700 (#1d4ed8) on a dark background would have very low contrast (~2–3:1), failing AA. Consider adding a dark-mode hover variant to maintain contrast:Suggested fix
- className="hover:text-blue-700 hover:underline dark:text-blue-300" + className="hover:text-blue-700 hover:underline dark:text-blue-300 dark:hover:text-blue-200"frontend/src/app/board/[year]/candidates/page.tsx (1)
348-358:⚠️ Potential issue | 🔴 CriticalBug:
text-blue-300in light mode severely fails WCAG AA contrast.On the white card background (
bg-whiteat Line 311),text-blue-300(#93c5fd) yields roughly a 2.4:1 contrast ratio — well below the 4.5:1 minimum this PR is trying to enforce. Every other link in this file uses thetext-blue-600 ... dark:text-blue-300pattern (e.g., Lines 705, 714, 733). This line is missing thedark:prefix and is using a light-mode color that's worse than what was there before.🐛 Proposed fix
<Link href={`https://github.com/${candidate.member.login}`} target="_blank" rel="noopener noreferrer" - className="truncate text-sm text-blue-300 hover:underline" + className="truncate text-sm text-blue-600 hover:underline dark:text-blue-300" onClick={(e) => e.stopPropagation()} >frontend/src/app/members/[memberKey]/page.tsx (1)
69-76:⚠️ Potential issue | 🟠 Major
text-blue-300fails WCAG AA contrast in light mode.Tailwind's
blue-300(#93c5fd) on white or light backgrounds yields a 1.80:1 contrast ratio—far below the 4.5:1 AA minimum. The previoustext-blue-400was also insufficient at 2.54:1.Use separate light/dark tokens so both modes pass:
Proposed fix
- className="text-blue-300 hover:underline" + className="text-blue-600 hover:underline dark:text-blue-300"Light mode:
text-blue-600achieves 5.17:1 on white backgrounds. Dark mode:dark:text-blue-300achieves 8.14:1 on dark backgrounds.
🤖 Fix all issues with AI agents
In `@frontend/src/app/about/page.tsx`:
- Around line 144-145: The text color class text-blue-300 fails WCAG AA on light
backgrounds across many components; update every bare occurrence of
"text-blue-300" (e.g., in the JSX line rendering feature.title in about/page.tsx
and in Card, RepositoryCard, SingleModuleCard, CardDetailsPage,
ContributorsList, ItemCardList, Release, ShowMoreButton, SnapshotCard, SortBy,
UserCard, ModuleCard, and app routes) to use a light-mode safe color with the
blue-300 shade only in dark mode — i.e., replace bare "text-blue-300" with a
combination that applies blue-700 for light mode and blue-300 for dark mode
(dark:text-blue-300 plus a light-mode class such as text-blue-700) everywhere it
appears.
In `@frontend/src/app/chapters/`[chapterKey]/page.tsx:
- Line 64: Replace the low-contrast Tailwind class "text-blue-300" with a
dark-mode-aware pair so light-mode uses a darker blue and dark-mode keeps the
lighter blue, e.g., change usages like the Link component's className from
"text-blue-300" to "text-blue-700 dark:text-blue-300"; search for other
occurrences (ItemCardList, Release, RepositoryCard, UserCard, SingleModuleCard,
ShowMoreButton, ModuleCard, chapters, organizations, committees, members pages,
etc.) and apply the same replacement to each component or element using
"text-blue-300".
In `@frontend/src/components/ItemCardList.tsx`:
- Line 64: The link text class "text-blue-300" in ItemCardList.tsx (the element
using className "shrink-0 text-blue-300 hover:underline") has insufficient
contrast on its parent "bg-gray-200"; replace it with separate light/dark color
classes so light mode uses a higher-contrast blue (e.g., a darker blue token for
light mode) while keeping the lighter blue for dark mode—update both occurrences
(the one shown and the similar use around line 83) to use conditional classes
like a darker blue for light and text-blue-300 for dark (e.g., text-blue-600 /
dark:text-blue-300) and keep the hover:underline behavior intact.
In `@frontend/src/components/Release.tsx`:
- Line 53: The link text color is too low-contrast for light mode; update the
Tailwind color classes to use a light/dark split that meets WCAG AA (prefer the
suggested combination): replace occurrences like className="shrink-0
text-blue-300 hover:underline" with a variant that uses text-blue-600 for light
and dark:text-blue-400 for dark (e.g., text-blue-600 dark:text-blue-400
hover:underline) in Release.tsx and apply the same pattern to the other affected
components (ItemCardList, ContributorsList, ModuleCard, ShowMoreButton,
SingleModuleCard, Card, RepositoryCard, UserCard, app/page) so links meet
contrast requirements across modes while preserving hover/underline behavior.
In `@frontend/src/components/RepositoryCard.tsx`:
- Line 61: Replace the low-contrast "text-blue-300" in the RepositoryCard
component's className with an accessible light-mode color and keep blue-300 for
dark mode (e.g., use "text-blue-700 dark:text-blue-300" or "text-blue-600
dark:text-blue-300"); update the className on the element inside
RepositoryCard.tsx that currently contains "text-blue-300" so the
higher-contrast blue is used in light mode and blue-300 only applies under the
dark: prefix, then run the provided verification script to confirm no other
occurrences of unprefixed "text-blue-300" remain.
🟠 Major comments (16)
frontend/src/components/Pagination.tsx-83-86 (1)
83-86:⚠️ Potential issue | 🟠 Major
bg-[#83a6cc]withtext-whitefails WCAG AA contrast (2.53:1; needs 4.5:1 minimum).The active-page button uses a contrast ratio of approximately 2.53:1, well below WCAG AA requirements. The dark mode pair (white background + black text) passes at 21:1, so the issue is light mode only.
The proposed color
bg-[#4a7ab5]yields only 4.43:1 and still fails. Either darken the background further (e.g., to achieve ≥4.5:1 with white text), or switch to dark text on the current background and adjust accordingly.frontend/src/app/members/[memberKey]/page.tsx-144-149 (1)
144-149:⚠️ Potential issue | 🟠 MajorSame
text-blue-300light-mode contrast issue as the bio mention links.This profile link has the same problem identified on Line 73 —
blue-300fails AA in light mode. Apply the same split-token fix:Proposed fix
- className="text-xl font-bold text-blue-300 hover:underline" + className="text-xl font-bold text-blue-600 hover:underline dark:text-blue-300"frontend/src/app/committees/[committeeKey]/page.tsx-65-65 (1)
65-65:⚠️ Potential issue | 🟠 Major
text-blue-300withoutdark:prefix regresses light-mode contrast — use conditional light/dark classes.On the white background of this CardDetailsPage,
text-blue-300(#93c5fd) delivers only ~1.80:1 contrast ratio, failing WCAG AA (requires 4.5:1) and worsening the priortext-blue-400(~2.54:1). Dark mode incorrectly benefits (5.72:1 ondark:bg-[#212529]), but applying the lighter blue unconditionally sacrifices light-mode accessibility.Split into light/dark variants:
text-blue-600achieves 5.17:1 on white (passes AA), whiledark:text-blue-300achieves 5.72:1 on dark backgrounds (passes AA). This pattern has been applied site-wide across multiple components (CardDetailsPage, RepositoryCard, SingleModuleCard, etc.); some files (IssuesTable, MentorshipPullRequest) already use the correct dual-class approach.Suggested fix
- <Link href={committee.url} className="text-blue-300 hover:underline"> + <Link href={committee.url} className="text-blue-600 hover:underline dark:text-blue-300">frontend/src/app/organizations/[organizationKey]/page.tsx-62-62 (1)
62-62:⚠️ Potential issue | 🟠 MajorSame light-mode contrast regression as the repository page link.
text-blue-300without adark:variant will have poor contrast on light backgrounds. Apply the same split-color fix here.Proposed fix
- <Link href={organization.url} className="text-blue-300 hover:underline"> + <Link href={organization.url} className="text-blue-600 hover:underline dark:text-blue-300">frontend/src/components/ShowMoreButton.tsx-19-19 (1)
19-19:⚠️ Potential issue | 🟠 Major
text-blue-300on transparent background risks poor contrast in light mode.Since the button uses
bg-transparent, the effective background depends on the parent. On light page backgrounds (white/light gray),blue-300will not meet 4.5:1. Use separate light/dark color tokens.Proposed fix
- className="flex items-center bg-transparent px-2 py-1 text-blue-300 hover:underline focus-visible:rounded focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-blue-500" + className="flex items-center bg-transparent px-2 py-1 text-blue-700 hover:underline focus-visible:rounded focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-blue-500 dark:text-blue-300"frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx-72-72 (1)
72-72:⚠️ Potential issue | 🟠 Major
text-blue-300fails WCAG AA contrast on light backgrounds.On the
bg-gray-100background used by SecondaryCard in light mode,text-blue-300(#93c5fd) has a contrast ratio of only 1.64:1, failing the WCAG AA minimum of 4.5:1. In dark mode onbg-gray-800, the same color achieves 8.14:1, so it needs a mode-aware approach.Use
text-blue-600for light mode anddark:text-blue-300for dark mode:Proposed fix
- <Link href={repository.url} className="text-blue-300 hover:underline"> + <Link href={repository.url} className="text-blue-600 hover:underline dark:text-blue-300">This pattern is already used elsewhere in the codebase (e.g.,
MentorshipPullRequest.tsx,IssuesTable.tsx).frontend/src/components/SingleModuleCard.tsx-92-92 (1)
92-92:⚠️ Potential issue | 🟠 Major
text-blue-300on light backgrounds (bg-gray-200,bg-gray-100) fails WCAG AA.Same issue as in
SnapshotCard.tsx—blue-300(#93c5fd) againstbg-gray-200(~1.8:1) andbg-gray-100(~2.2:1) is well below the 4.5:1 minimum and is a regression from the previousblue-400.Use a dark blue for light mode and a light blue for dark mode:
Proposed fix
- className="cursor-pointer overflow-hidden font-semibold text-ellipsis whitespace-nowrap text-blue-300 hover:underline" + className="cursor-pointer overflow-hidden font-semibold text-ellipsis whitespace-nowrap text-blue-700 hover:underline dark:text-blue-300"- <h2 className="max-w-full text-2xl font-semibold break-words text-blue-300 transition-colors duration-300 hover:text-blue-600 sm:break-normal"> + <h2 className="max-w-full text-2xl font-semibold break-words text-blue-700 transition-colors duration-300 hover:text-blue-600 dark:text-blue-300 dark:hover:text-blue-400 sm:break-normal">Also applies to: 117-117
frontend/src/components/SnapshotCard.tsx-13-13 (1)
13-13:⚠️ Potential issue | 🟠 Major
text-blue-300onbg-whitefails WCAG AA contrast requirement.Tailwind's
text-blue-300(#93C5FD) on white yields a 1.80:1 contrast ratio, below the required 4.5:1 for normal text. Line 13 (hover state) and line 27 ("View Snapshot" button text) both use this combination on light backgrounds. Dark mode withtext-blue-300ondark:bg-gray-800is adequate (8.14:1), but light mode requires a darker blue.Apply separate light/dark classes on both affected lines:
- Line 13:
text-gray-900 group-hover:text-blue-700 dark:group-hover:text-blue-300- Line 27:
text-blue-700 dark:text-blue-300
text-blue-700(#1D4ED8) on white achieves 6.70:1, meeting WCAG AA.frontend/src/components/UserCard.tsx-86-86 (1)
86-86:⚠️ Potential issue | 🟠 MajorSame light-mode contrast regression: bare
text-blue-300on a light background.As flagged in
about/page.tsx,text-blue-300(~#93C5FD) on the card's light background fails WCAG AA. The button label needs a dark blue for light mode.Proposed fix
- <div className="flex items-center justify-center text-sm font-medium text-blue-300"> + <div className="flex items-center justify-center text-sm font-medium text-blue-700 dark:text-blue-300">frontend/src/components/Card.tsx-57-57 (1)
57-57:⚠️ Potential issue | 🟠 Major
text-blue-300in the card title fails light-mode contrast — same issue flagged in other files.The title link starts at
text-blue-300(~#93C5FD) on the card'sbg-white(line 32), giving ~2.2:1 contrast — well below AA thresholds even for large text. The hover state (hover:text-blue-600) is fine, but the default state is the one most users see.Proposed fix
- className="max-w-full text-base font-semibold break-words text-blue-300 hover:text-blue-600 sm:text-lg sm:break-normal lg:text-2xl" + className="max-w-full text-base font-semibold break-words text-blue-700 hover:text-blue-600 sm:text-lg sm:break-normal lg:text-2xl dark:text-blue-300 dark:hover:text-blue-100"frontend/src/components/ContributorsList.tsx-62-65 (1)
62-65:⚠️ Potential issue | 🟠 Major
text-blue-300onbg-gray-200fails WCAG AA contrast in light mode (1.46:1, needs 4.5:1).The change from
text-blue-400totext-blue-300improves dark-mode contrast (5.72:1 onbg-gray-700) but creates a regression in light mode.text-blue-300(#93C5FD) produces only 1.46:1 againstbg-gray-200(#E5E7EB)—below the AA threshold and worse than the previousblue-400(2.05:1).Use separate light/dark variants to pass AA in both modes:
Proposed fix
<Link - className="cursor-pointer overflow-hidden font-semibold text-ellipsis whitespace-nowrap text-blue-300 hover:underline" + className="cursor-pointer overflow-hidden font-semibold text-ellipsis whitespace-nowrap text-blue-700 hover:underline dark:text-blue-300" href={getUrl(item?.login)} title={item?.name || item?.login} >
text-blue-700(#1D4ED8) onbg-gray-200gives 5.41:1 anddark:text-blue-300ondark:bg-gray-700gives 5.72:1—both pass AA.frontend/src/app/page.tsx-382-382 (1)
382-382:⚠️ Potential issue | 🟠 MajorStat counter
text-blue-300also fails the large-text AA threshold (3:1).Even though these are
text-3xl font-bold(qualifying as large text under WCAG, requiring only 3:1), blue-300 on a light background yields ~1.8:1, which still fails. Consider usingtext-blue-600 dark:text-blue-300here as well.frontend/src/app/page.tsx-170-170 (1)
170-170:⚠️ Potential issue | 🟠 Major
text-blue-300onbg-gray-200cards fails WCAG AA in light mode (~1.5:1).All four link locations sit on
bg-gray-200in light mode. Same issue as flagged inModuleCard.tsxandCardDetailsPage.tsx— blue-300 is too light for any light background. A dual-token approach is needed:Proposed fix (apply to each occurrence)
- className="... text-blue-300 hover:underline" + className="... text-blue-600 hover:underline dark:text-blue-300"Also applies to: 231-231, 274-274, 357-357
frontend/src/components/CardDetailsPage.tsx-426-426 (1)
426-426:⚠️ Potential issue | 🟠 Major
text-blue-300fails WCAG AA in light mode onbg-gray-200(~1.5:1).These milestone links sit on
bg-gray-200in light mode.text-blue-300(#93C5FD) onbg-gray-200(#E5E7EB) produces a contrast ratio of roughly 1.5:1 — far below the 4.5:1 requirement. The dark-mode contrast (ondark:bg-gray-700) is fine at ~6.4:1.Same root cause as flagged in
ModuleCard.tsx— needs a dark-enough light-mode blue paired withdark:text-blue-300.Proposed fix
- className="shrink-0 text-blue-300 hover:underline" + className="shrink-0 text-blue-600 hover:underline dark:text-blue-300"- className="text-blue-300 hover:underline" + className="text-blue-600 hover:underline dark:text-blue-300"Also applies to: 447-447
frontend/src/components/CardDetailsPage.tsx-526-526 (1)
526-526:⚠️ Potential issue | 🟠 Major
text-blue-300on the page's white background fails AA in light mode (~1.8:1).Social icon links inherit the main container's
bg-whitein light mode. Same root cause as the milestone links above.Proposed fix
- className="text-blue-300 transition-colors hover:text-gray-800 dark:hover:text-gray-200" + className="text-blue-600 transition-colors hover:text-gray-800 dark:text-blue-300 dark:hover:text-gray-200"frontend/src/components/ModuleCard.tsx-49-49 (1)
49-49:⚠️ Potential issue | 🟠 Major
text-blue-300fails WCAG AA in light mode (~1.8:1 on white).Unlike
IssuesTable.tsx(line 127), which correctly usestext-blue-600 dark:text-blue-300to pass AA in both modes, this button uses a singletext-blue-300for both. Tailwind blue-300 (#93C5FD) against white orbg-gray-200yields a contrast ratio well below 4.5:1.Consider splitting the token so light mode uses a sufficiently dark blue:
Proposed fix
- className="mt-4 flex items-center justify-center text-blue-300 hover:underline focus:rounded focus:outline-2 focus:outline-offset-2 focus:outline-blue-500" + className="mt-4 flex items-center justify-center text-blue-600 hover:underline focus:rounded focus:outline-2 focus:outline-offset-2 focus:outline-blue-500 dark:text-blue-300"
🟡 Minor comments (4)
frontend/src/components/BreadCrumbs.tsx-32-32 (1)
32-32:⚠️ Potential issue | 🟡 MinorInconsistent dark-mode text shade:
dark:text-gray-300vsdark:text-gray-200.The PR objectives and most other files in this changeset standardize on
dark:text-gray-200as the replacement fordark:text-gray-400. This line usesdark:text-gray-300instead. While gray-300 still passes AA on typical dark backgrounds, it's inconsistent with the rest of the PR.Suggested fix
- className="cursor-default font-semibold text-gray-800 dark:text-gray-300" + className="cursor-default font-semibold text-gray-800 dark:text-gray-200"frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/page.tsx-62-64 (1)
62-64:⚠️ Potential issue | 🟡 MinorBiome flags: arrow-function bodies in
forEachcallbacks implicitly return a value.The arrow
=>without braces makes each callback an expression-body that returns the innerforEachresult. While harmless at runtime (return value is ignored), this triggers theuseIterableCallbackReturnlint rule and the leading;on line 62 is an unusual ASI-guard that hurts readability.Proposed fix — use block bodies to silence the lint and improve clarity
- ; (moduleData?.issues || []).forEach((i) => - (i.labels || []).forEach((l: string) => labels.add(l)) - ) + ;(moduleData?.issues || []).forEach((i) => { + (i.labels || []).forEach((l: string) => { + labels.add(l) + }) + })frontend/src/components/MentorshipPullRequest.tsx-47-47 (1)
47-47:⚠️ Potential issue | 🟡 MinorDark-mode hover state is identical to the base state — no visual feedback.
dark:text-blue-300anddark:hover:text-blue-300resolve to the same color, so hovering the link provides zero visual feedback in dark mode. This likely happened whendark:text-blue-400was bumped todark:text-blue-300without also adjusting the hover variant.Proposed fix
- className="font-medium text-blue-600 hover:text-blue-800 dark:text-blue-300 dark:hover:text-blue-300" + className="font-medium text-blue-600 hover:text-blue-800 dark:text-blue-300 dark:hover:text-blue-100"frontend/src/components/MentorshipPullRequest.tsx-51-53 (1)
51-53:⚠️ Potential issue | 🟡 Minortext-gray-500 on bg-gray-200 falls short of WCAG AA contrast requirements.
text-gray-500(#6B7280) onbg-gray-200(#E5E7EB) produces a 4.10:1 contrast ratio, which is below the 4.5:1 AA threshold for normal text. Since this was introduced in a PR specifically targeting WCAG compliance, consider upgrading totext-gray-700(4.92:1) ortext-gray-800(6.20:1) for consistency with the PR's accessibility goals.
🧹 Nitpick comments (6)
frontend/src/app/board/[year]/candidates/page.tsx (1)
341-341: No hover feedback for LinkedIn icon in dark mode.
dark:text-blue-300 dark:hover:text-blue-300are identical, so the icon has no visual change on hover in dark mode. Consider using a slightly different shade (e.g.,dark:hover:text-blue-200) for interactive affordance.frontend/src/app/members/[memberKey]/page.tsx (1)
143-143: Dark mode fix looks correct; light-modetext-gray-500should be verified.The
dark:text-gray-200update is a good improvement. However,text-gray-500(#6b7280) passes AA on pure white (~5.9:1) but may not on slightly tinted or gray backgrounds. If the parent card has a non-white light-mode background, consider bumping totext-gray-700for more headroom.frontend/src/components/forms/shared/FormTextInput.tsx (1)
5-12: LGTM!Label contrast improved to
text-gray-800in light mode. TheCOMMON_INPUT_CLASS_NAMESobject here is now identical to the one inFormDateInput.tsx— consider extracting it into a shared module to keep them in sync, but that's a pre-existing concern and not blocking.frontend/src/components/forms/shared/FormTextarea.tsx (1)
43-44: Minor formatting nit: awkward line break inside template literal.The closing
}of the ternary sits alone on Line 44 with extra indentation. Consider reformatting to keep the template literal more readable.🔧 Suggested formatting
- className={`w-full min-w-0 rounded-lg border px-3 py-2 text-gray-800 placeholder:text-gray-400 focus:border-[`#1D7BD7`] focus:ring-1 focus:ring-[`#1D7BD7`] focus:outline-none dark:bg-gray-800 dark:text-gray-200 dark:focus:ring-[`#1D7BD7`] ${hasError ? 'border-red-500 dark:border-red-500' : 'border-gray-300 dark:border-gray-600' - }`} + className={`w-full min-w-0 rounded-lg border px-3 py-2 text-gray-800 placeholder:text-gray-400 focus:border-[`#1D7BD7`] focus:ring-1 focus:ring-[`#1D7BD7`] focus:outline-none dark:bg-gray-800 dark:text-gray-200 dark:focus:ring-[`#1D7BD7`] ${hasError ? 'border-red-500 dark:border-red-500' : 'border-gray-300 dark:border-gray-600'}`}frontend/src/components/Modal.tsx (1)
43-43: Contrast improvements look good; minor dark-mode inconsistency between summary and hint sections.Both sections now use
text-gray-800in light mode (good). In dark mode, the summary usesdark:text-gray-300(Line 43) while the hint usesdark:text-gray-200(Line 55). Both pass AA, but the inconsistency within the same modal may be unintentional — consider aligning them to the same shade.🔧 Align dark-mode text color for both Markdown sections
<Markdown content={summary} - className="text-base [overflow-wrap:anywhere] break-words text-gray-800 dark:text-gray-300" + className="text-base [overflow-wrap:anywhere] break-words text-gray-800 dark:text-gray-200" />Also applies to: 55-55
frontend/src/components/SnapshotCard.tsx (1)
18-18: Minor inconsistency:dark:text-gray-300instead ofdark:text-gray-200.All other files in this PR use
dark:text-gray-200for dark-mode text, but this line usesdark:text-gray-300. Both pass WCAG AA ondark:bg-gray-800, but if the intent is a consistent token across the codebase, consider aligning todark:text-gray-200.
| label: 'URL', | ||
| value: ( | ||
| <Link href={chapter.url} className="text-blue-400 hover:underline"> | ||
| <Link href={chapter.url} className="text-blue-300 hover:underline"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and read the specific file to confirm line 64
find . -type f -name "page.tsx" -path "*chapters*" | head -5Repository: OWASP/Nest
Length of output: 140
🏁 Script executed:
# Search for text-blue-300 instances in the codebase
rg -n 'text-blue-300' --type=tsx --type=ts --type=jsx -B 2 -A 2Repository: OWASP/Nest
Length of output: 81
🏁 Script executed:
# Search for text-blue-400 to see what was replaced
rg -n 'text-blue-400' --type=tsx --type=ts --type=jsxRepository: OWASP/Nest
Length of output: 81
🏁 Script executed:
# Check the specific file mentioned in the review
cat -n frontend/src/app/chapters/[chapterKey]/page.tsx | sed -n '55,75p'Repository: OWASP/Nest
Length of output: 752
🏁 Script executed:
# Search for text-blue-300 without type restriction
rg 'text-blue-300' -n --glob '*.tsx' --glob '*.jsx' --glob '*.ts' --glob '*.js'Repository: OWASP/Nest
Length of output: 9563
🏁 Script executed:
# Search for text-blue-400 to understand what was changed
rg 'text-blue-400' -n --glob '*.tsx' --glob '*.jsx' --glob '*.ts' --glob '*.js'Repository: OWASP/Nest
Length of output: 36
🏁 Script executed:
# Check git history for this specific file to see what changed
git diff HEAD~1 frontend/src/app/chapters/[chapterKey]/page.tsx 2>/dev/null || echo "Git diff not available"Repository: OWASP/Nest
Length of output: 76
🏁 Script executed:
# Check the ActionButton.tsx file mentioned in learnings to understand design patterns
cat frontend/src/components/ActionButton.tsx | grep -n 'text-\|className' | head -20Repository: OWASP/Nest
Length of output: 335
text-blue-300 on light backgrounds causes WCAG AA contrast failure — this is widespread across 54+ files.
Tailwind's blue-300 (#93c5fd) on white yields approximately 2.8:1 contrast ratio, well below the 4.5:1 AA requirement. The class is applied unconditionally (no dark: prefix) across ItemCardList, Release, RepositoryCard, UserCard, SingleModuleCard, ShowMoreButton, ModuleCard, chapters, organizations, committees, members pages and many others, meaning it affects light mode.
Use a dark variant for light mode and the lighter blue for dark mode:
- <Link href={chapter.url} className="text-blue-300 hover:underline">
+ <Link href={chapter.url} className="text-blue-600 hover:underline dark:text-blue-300">This pattern requires fixing across the entire PR — not just this one file.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Link href={chapter.url} className="text-blue-300 hover:underline"> | |
| <Link href={chapter.url} className="text-blue-600 hover:underline dark:text-blue-300"> |
🤖 Prompt for AI Agents
In `@frontend/src/app/chapters/`[chapterKey]/page.tsx at line 64, Replace the
low-contrast Tailwind class "text-blue-300" with a dark-mode-aware pair so
light-mode uses a darker blue and dark-mode keeps the lighter blue, e.g., change
usages like the Link component's className from "text-blue-300" to
"text-blue-700 dark:text-blue-300"; search for other occurrences (ItemCardList,
Release, RepositoryCard, UserCard, SingleModuleCard, ShowMoreButton, ModuleCard,
chapters, organizations, committees, members pages, etc.) and apply the same
replacement to each component or element using "text-blue-300".
frontend/src/components/Release.tsx
Outdated
| > | ||
| <Link | ||
| className="shrink-0 text-blue-400 hover:underline" | ||
| className="shrink-0 text-blue-300 hover:underline" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the Release.tsx file around lines 53 and 72
head -100 frontend/src/components/Release.tsx | tail -60Repository: OWASP/Nest
Length of output: 2710
🏁 Script executed:
# Check the git diff to see what was changed
git diff HEAD frontend/src/components/Release.tsx | head -100Repository: OWASP/Nest
Length of output: 36
🏁 Script executed:
# Find all text-blue-300 instances without dark: prefix
rg 'text-blue-300(?!.*dark:)' --type=tsx frontend/src/components/ -A 1 -B 1Repository: OWASP/Nest
Length of output: 294
🏁 Script executed:
# Let's also verify the exact color values from Tailwind config
# and calculate contrast ratio properly
python3 << 'EOF'
from colorsys import rgb_to_hls
def hex_to_rgb(hex_color):
hex_color = hex_color.lstrip('#')
return tuple(int(hex_color[i:i+2], 16) for i in (0, 2, 4))
def relative_luminance(r, g, b):
"""Calculate relative luminance per WCAG formula"""
r, g, b = r/255.0, g/255.0, b/255.0
r = r/12.92 if r <= 0.03928 else ((r+0.055)/1.055)**2.4
g = g/12.92 if g <= 0.03928 else ((g+0.055)/1.055)**2.4
b = b/12.92 if b <= 0.03928 else ((b+0.055)/1.055)**2.4
return 0.2126*r + 0.7152*g + 0.0722*b
def contrast_ratio(color1, color2):
"""Calculate contrast ratio between two hex colors"""
r1, g1, b1 = hex_to_rgb(color1)
r2, g2, b2 = hex_to_rgb(color2)
l1 = relative_luminance(r1, g1, b1)
l2 = relative_luminance(r2, g2, b2)
lighter = max(l1, l2)
darker = min(l1, l2)
return (lighter + 0.05) / (darker + 0.05)
# Tailwind colors
gray_200 = '#e5e7eb' # bg-gray-200
gray_700 = '#374151' # dark:bg-gray-700
blue_300 = '#93c5fd' # text-blue-300
blue_400 = '#60a5fa' # text-blue-400
blue_600 = '#2563eb' # text-blue-600
blue_700 = '#1d4ed8' # text-blue-700
# Test original claim
ratio_blue300_on_gray200 = contrast_ratio(blue_300, gray_200)
ratio_blue400_on_gray200 = contrast_ratio(blue_400, gray_200)
ratio_blue600_on_gray200 = contrast_ratio(blue_600, gray_200)
ratio_blue700_on_gray200 = contrast_ratio(blue_700, gray_200)
print(f"text-blue-300 (`#93c5fd`) on bg-gray-200 (`#e5e7eb`): {ratio_blue300_on_gray200:.2f}:1")
print(f"text-blue-400 (`#60a5fa`) on bg-gray-200 (`#e5e7eb`): {ratio_blue400_on_gray200:.2f}:1")
print(f"text-blue-600 (`#2563eb`) on bg-gray-200 (`#e5e7eb`): {ratio_blue600_on_gray200:.2f}:1")
print(f"text-blue-700 (`#1d4ed8`) on bg-gray-200 (`#e5e7eb`): {ratio_blue700_on_gray200:.2f}:1")
print()
# Dark mode
ratio_blue300_on_gray700 = contrast_ratio(blue_300, gray_700)
ratio_blue400_on_gray700 = contrast_ratio(blue_400, gray_700)
print(f"text-blue-300 (`#93c5fd`) on dark:bg-gray-700 (`#374151`): {ratio_blue300_on_gray700:.2f}:1")
print(f"text-blue-400 (`#60a5fa`) on dark:bg-gray-700 (`#374151`): {ratio_blue400_on_gray700:.2f}:1")
print()
print(f"WCAG AA requires 4.5:1 minimum for normal text")
print(f"WCAG AAA requires 7:1 minimum for normal text")
EOFRepository: OWASP/Nest
Length of output: 500
🏁 Script executed:
# Check parent container - look at more context above line 40
head -60 frontend/src/components/Release.tsx | cat -nRepository: OWASP/Nest
Length of output: 2497
🏁 Script executed:
# Find all text-blue-300 instances using PCRE2 flag for look-ahead
rg 'text-blue-300' --type=tsx frontend/src/ -B 2 -A 2Repository: OWASP/Nest
Length of output: 81
🏁 Script executed:
# Check if there's a parent container with bg-gray-200 in Release.tsx
rg 'bg-gray-200' frontend/src/components/Release.tsx -B 5 -A 5Repository: OWASP/Nest
Length of output: 408
🏁 Script executed:
# Find all text-blue-300 instances in the frontend
rg 'text-blue-300' frontend/src/ -A 2 -B 2Repository: OWASP/Nest
Length of output: 27495
🏁 Script executed:
# Also check git log to see if there's context about this change
git log --oneline -20 -- frontend/src/components/Release.tsxRepository: OWASP/Nest
Length of output: 116
🏁 Script executed:
# Check if there are recent changes showing blue-400 → blue-300 pattern
git diff HEAD~5 HEAD frontend/src/components/ | grep -A 3 -B 3 'blue-[34]00' | head -60Repository: OWASP/Nest
Length of output: 240
🏁 Script executed:
# Let's check if there's a parent container context that might affect the background
rg 'bg-gray-200|dark:bg-gray-700' frontend/src/components/Release.tsx -B 5 -A 10 | head -40Repository: OWASP/Nest
Length of output: 604
🏁 Script executed:
# Let's look at what files with text-blue-300 have bg-gray-200 containers
# to understand the scope of the issue
rg 'text-blue-300' frontend/src/app/page.tsx -B 10 | grep -E '(bg-gray|text-blue-300)'Repository: OWASP/Nest
Length of output: 864
🏁 Script executed:
# Verify if the proposed fix in the review would work by checking light/dark contrast
python3 << 'EOF'
def hex_to_rgb(hex_color):
hex_color = hex_color.lstrip('#')
return tuple(int(hex_color[i:i+2], 16) for i in (0, 2, 4))
def relative_luminance(r, g, b):
r, g, b = r/255.0, g/255.0, b/255.0
r = r/12.92 if r <= 0.03928 else ((r+0.055)/1.055)**2.4
g = g/12.92 if g <= 0.03928 else ((g+0.055)/1.055)**2.4
b = b/12.92 if b <= 0.03928 else ((b+0.055)/1.055)**2.4
return 0.2126*r + 0.7152*g + 0.0722*b
def contrast_ratio(color1, color2):
r1, g1, b1 = hex_to_rgb(color1)
r2, g2, b2 = hex_to_rgb(color2)
l1 = relative_luminance(r1, g1, b1)
l2 = relative_luminance(r2, g2, b2)
lighter = max(l1, l2)
darker = min(l1, l2)
return (lighter + 0.05) / (darker + 0.05)
gray_200 = '#e5e7eb'
gray_700 = '#374151'
blue_700 = '#1d4ed8'
print("Proposed fix verification:")
print(f"text-blue-700 (`#1d4ed8`) on bg-gray-200 (`#e5e7eb`): {contrast_ratio(blue_700, gray_200):.2f}:1")
print(f"text-blue-700 (`#1d4ed8`) on dark:bg-gray-700 (`#374151`): {contrast_ratio(blue_700, gray_700):.2f}:1")
print()
print("Both exceed 4.5:1 WCAG AA minimum ✓" if contrast_ratio(blue_700, gray_200) >= 4.5 and contrast_ratio(blue_700, gray_700) >= 4.5 else "At least one fails")
EOFRepository: OWASP/Nest
Length of output: 219
🏁 Script executed:
# The proposed fix doesn't work - blue-700 fails in dark mode.
# Let's find what blue shades work in both modes
python3 << 'EOF'
def hex_to_rgb(hex_color):
hex_color = hex_color.lstrip('#')
return tuple(int(hex_color[i:i+2], 16) for i in (0, 2, 4))
def relative_luminance(r, g, b):
r, g, b = r/255.0, g/255.0, b/255.0
r = r/12.92 if r <= 0.03928 else ((r+0.055)/1.055)**2.4
g = g/12.92 if g <= 0.03928 else ((g+0.055)/1.055)**2.4
b = b/12.92 if b <= 0.03928 else ((b+0.055)/1.055)**2.4
return 0.2126*r + 0.7152*g + 0.0722*b
def contrast_ratio(color1, color2):
r1, g1, b1 = hex_to_rgb(color1)
r2, g2, b2 = hex_to_rgb(color2)
l1 = relative_luminance(r1, g1, b1)
l2 = relative_luminance(r2, g2, b2)
lighter = max(l1, l2)
darker = min(l1, l2)
return (lighter + 0.05) / (darker + 0.05)
gray_200 = '#e5e7eb'
gray_700 = '#374151'
blues = {
'blue-500': '#3b82f6',
'blue-600': '#2563eb',
'blue-700': '#1d4ed8',
'blue-400': '#60a5fa',
'blue-300': '#93c5fd',
}
print("Finding blues that work in BOTH light and dark modes (≥4.5:1):\n")
print(f"{'Blue shade':<15} {'Light (gray-200)':<20} {'Dark (gray-700)':<20} {'Both OK?':<10}")
print("-" * 65)
for shade, color in blues.items():
light_ratio = contrast_ratio(color, gray_200)
dark_ratio = contrast_ratio(color, gray_700)
both_ok = "✓" if light_ratio >= 4.5 and dark_ratio >= 4.5 else "✗"
print(f"{shade:<15} {light_ratio:>6.2f}:1 {'✓' if light_ratio >= 4.5 else '✗':<12} {dark_ratio:>6.2f}:1 {'✓' if dark_ratio >= 4.5 else '✗':<12} {both_ok:<10}")
EOFRepository: OWASP/Nest
Length of output: 607
🏁 Script executed:
# Check if there are any instances with proper dark: variants already in the codebase
# that show the correct pattern
rg 'text-blue.*dark:text-blue' frontend/src/ -A 1 -B 1 | head -30Repository: OWASP/Nest
Length of output: 2675
text-blue-300 on bg-gray-200 fails WCAG AA in light mode — text is barely distinguishable.
Confirmed: text-blue-300 (#93c5fd) on bg-gray-200 (#e5e7eb) yields a contrast ratio of 1.46:1, well below the 4.5:1 minimum. This is a critical accessibility issue affecting actual text links, not just images.
However, there is a fundamental constraint: no single Tailwind blue shade achieves 4.5:1 contrast in both light mode (gray-200) and dark mode (gray-700). The design requires a split approach:
- Light mode needs blue-600+ (4.17:1 minimum)
- Dark mode needs blue-300+ (5.72:1 minimum)
The proposed text-blue-700 dark:text-blue-300 fix fails in dark mode (1.54:1), introducing a regression worse than the original blue-400 (4.05:1).
Proper solutions:
- Use a light/dark variant that prioritizes light mode:
text-blue-600 dark:text-blue-400 - Change the container backgrounds to enable better contrast with a single blue
- Adopt a consistent theme strategy across light and dark modes
This issue spans multiple components (Release.tsx, ItemCardList.tsx, ContributorsList.tsx, ModuleCard.tsx, ShowMoreButton.tsx, SingleModuleCard.tsx, Card.tsx, RepositoryCard.tsx, UserCard.tsx, and app/page.tsx).
🤖 Prompt for AI Agents
In `@frontend/src/components/Release.tsx` at line 53, The link text color is too
low-contrast for light mode; update the Tailwind color classes to use a
light/dark split that meets WCAG AA (prefer the suggested combination): replace
occurrences like className="shrink-0 text-blue-300 hover:underline" with a
variant that uses text-blue-600 for light and dark:text-blue-400 for dark (e.g.,
text-blue-600 dark:text-blue-400 hover:underline) in Release.tsx and apply the
same pattern to the other affected components (ItemCardList, ContributorsList,
ModuleCard, ShowMoreButton, SingleModuleCard, Card, RepositoryCard, UserCard,
app/page) so links meet contrast requirements across modes while preserving
hover/underline behavior.
arkid15r
left a comment
arkid15r
left a comment
There was a problem hiding this comment.
Please follow our contributing workflow and use our standard PR template. https://github.com/OWASP/Nest/blob/main/CONTRIBUTING.md#contributing-workflow
|
Looks like most of these are now failing WCAG AA rather than getting fixed 👀 |
|
Thank you for the detailed review @kasya and @arkid15r. |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/components/CardDetailsPage.tsx (1)
521-527:⚠️ Potential issue | 🔴 Critical
text-blue-300onbg-whitefails WCAG AA in light mode — same regression as the avatar link.The
SocialLinkscomponent inherits thebg-whitemain container background (line 136). Tailwind'sblue-300(#93c5fd) on white yields roughly 2.5:1 contrast — well below the 4.5:1 requirement. A dark-mode qualifier is needed here just like on line 447.Also note the hover states:
hover:text-gray-800is fine for light mode, butdark:hover:text-gray-200only styles the hover. The default state in dark mode is stillblue-300, which needs to be verified againstdark:bg-[#212529](likely passes, ~6:1), so only the light-mode base color needs fixing.Proposed fix
className="text-blue-300 transition-colors hover:text-gray-800 dark:hover:text-gray-200" + className="text-blue-700 transition-colors hover:text-blue-900 dark:text-blue-300 dark:hover:text-gray-200"frontend/__tests__/unit/pages/About.test.tsx (1)
1-9:⚠️ Potential issue | 🔴 CriticalAdd missing imports and define missing mock data.
The test uses
GetProjectMetadataDocument,GetTopContributorsDocument,GetLeaderDataDocument,mockProjectData,mockUserData, andmockTopContributorsDatawithout importing or defining them. This will causeReferenceErrorandTypeErrorfailures at runtime across multiple test cases (lines 203–221, 462–520, 585, 611, 697–711).Additionally,
mockUserDatais called as a functionmockUserData(key)on lines 215, 466, 520, and 709, but the exported mock is a plain object—this will fail withTypeError: mockUserData is not a function.🔧 Required changes
- Add missing imports:
import { useQuery } from '@apollo/client/react' import { addToast } from '@heroui/toast' import { mockAboutData } from '@mockData/mockAboutData' +import { mockProjectData } from '@mockData/mockProjectData' +import { mockUserData } from '@mockData/mockUserData' import { fireEvent, screen, waitFor, within } from '@testing-library/react' import { useRouter } from 'next/navigation' import React, { act } from 'react' import { render } from 'wrappers/testUtil' import About from 'app/about/page' import { GetAboutPageDataDocument } from 'types/__generated__/aboutQueries.generated' +import { GetProjectMetadataDocument, GetTopContributorsDocument } from 'types/__generated__/projectQueries.generated' +import { GetLeaderDataDocument } from 'types/__generated__/userQueries.generated'
- Create
mockTopContributorsData(missing from codebase) or define it in the test file:+import { mockAboutData } from '@mockData/mockAboutData' + +const mockTopContributorsData = mockAboutData.topContributors
- Fix
mockUserDatausage: either convert it to a factory function that returns user data by key, or usemockAboutData.users[key]directly.
🤖 Fix all issues with AI agents
In `@frontend/__tests__/unit/pages/About.test.tsx`:
- Line 177: The test uses a mock string with className "flex items-center
bg-transparent px-0 text-blue-300" which mirrors the component but uses a
too-light color; update the component's class usage and the test mock so the
light-mode text class is a darker shade (e.g., replace text-blue-300 with
text-blue-600 or text-blue-700) and keep the lighter color only as a dark:
variant (e.g., add dark:text-blue-300), then update the About.test.tsx mock to
match the new className string so the test reflects the accessible styling
change.
- Around line 635-695: Remove the duplicated Jest tests in the About.test.tsx
file by deleting the repeated test blocks with these names: "renders mission and
who its for sections correctly", "renders mission section", "renders 'Who It's
For' section", "renders key features section" (or "renders key features section
correctly"), "renders get involved section" (or "renders get involved section
correctly"), and "renders project history timeline section" (or "renders project
history timeline correctly"); keep only the original instances of these tests
(the first occurrences) so each assertion set (render(<About />) +
screen.getByText(...) expectations) appears exactly once.
- Around line 560-571: The mock implementation for useQuery inside the test
'renders ErrorDisplay when project is null' contains an unreachable final
"return { loading: true }" because the prior branches already return
unconditionally; remove that dead return (or restructure the mockImplementation
to use if/else if/else and only return once) so that useQuery mock only returns
the intended objects; reference the test name and the useQuery
mockImplementation to locate and fix the dead code.
- Around line 203-221: The beforeEach mock for useQuery references undefined
symbols from other pages; replace that mockImplementation to only handle
useQuery(GetAboutPageDataDocument) by reading variables.projectKey (and other
variables like excludedUsernames and leader usernames if present) and return the
imported mockAboutData, remove any branches for
GetProjectMetadataDocument/GetTopContributorsDocument/GetLeaderDataDocument and
references to mockProjectData/mockTopContributorsData/mockUserData, and fix the
malformed whitespace before the useRouter mock by ensuring mockRouter is created
(mockRouter = { push: jest.fn() }) and then calling (useRouter as
jest.Mock).mockReturnValue(mockRouter) without the stray leading semicolon.
In
`@frontend/src/app/organizations/`[organizationKey]/repositories/[repositoryKey]/page.tsx:
- Line 68: Replace the low-contrast Tailwind class on link elements so they meet
WCAG AA: in this file update the Link that renders repository.url (and all other
link components across chapters, committees, members, organizations, homepage)
to use "text-blue-600 dark:text-blue-300" instead of the bare "text-blue-300";
search for className="text-blue-300" (e.g., the Link in page.tsx) and change it
to include the dark: variant to preserve current dark-mode styling while
improving light-background contrast.
In `@frontend/src/components/CardDetailsPage.tsx`:
- Around line 425-427: The link showing the milestone author currently uses
text-blue-300 unconditionally which fails contrast in light mode; update the
Link rendering that targets `/members/${milestone?.author?.login}` (the Link
element around milestone?.author?.login) to use a dual-mode class like
"text-blue-700 dark:text-blue-300" (preserving existing classes such as
"shrink-0" and "hover:underline") so light mode uses an accessible darker blue
and dark mode keeps the lighter blue.
🧹 Nitpick comments (3)
frontend/__tests__/unit/pages/About.test.tsx (3)
462-469: Null-project test doesn't account forGetTopContributorsDocumentorGetProjectMetadataDocumentrouting.The mock on lines 462–468 only branches on
options?.variables?.key === 'nest'without checkingquery, so it returns{ data: { project: null }, ... }for bothGetProjectMetadataDocumentandGetTopContributorsDocumentwhen key is'nest'. If the component handles these independently, the test may pass for the wrong reason. Compare with thebeforeEachmock that branches onqueryfirst.♻️ Suggested improvement — branch on query type
;(useQuery as unknown as jest.Mock).mockImplementation((query, options) => { - if (options?.variables?.key === 'nest') { + if (query === GetProjectMetadataDocument && options?.variables?.key === 'nest') { return { data: { project: null }, loading: false, error: null } + } else if (query === GetTopContributorsDocument && options?.variables?.key === 'nest') { + return mockTopContributorsData } else if (['arkid15r', 'kasya', 'mamicidal'].includes(options?.variables?.key)) { return mockUserData(options?.variables?.key) } return { loading: true } })
583-607: Error tests forprojectandtopContributorsreturn dummy user data for all non-matching queries, hiding potential issues.Both mocks fall through to returning
{ data: { user: { ... } }, ... }for every other query, includingGetProjectMetadataDocumentandGetTopContributorsDocument(whichever isn't being tested). This means the component receives a response shape (data.user) that doesn't match the expected schema for those queries, which may silently mask bugs. Consider returning realistic mock data for the non-error queries so the test better reflects real behavior.Also applies to: 609-633
697-728: Leader error test: good coverage, but the duplicateGetLeaderDataDocumentbranch (lines 699 vs 708) could be fragile.The mock correctly short-circuits for
arkid15rwith an error on line 699, then falls through to a secondGetLeaderDataDocumentcheck on line 708 for other leaders. This works because the firstifmatches before the second, but it relies on ordering and makes the intent unclear. A single branch with a conditional would be clearer.♻️ Consolidate the leader branches
;(useQuery as unknown as jest.Mock).mockImplementation((query, options) => { - if (query === GetLeaderDataDocument && options?.variables?.key === 'arkid15r') { - return { loading: false, data: null, error: new Error('GraphQL error for leader') } - } - if (query === GetProjectMetadataDocument) { + if (query === GetProjectMetadataDocument) { return mockProjectData } if (query === GetTopContributorsDocument) { return mockTopContributorsData } - if (query === GetLeaderDataDocument) { - return mockUserData(options?.variables?.key) + if (query === GetLeaderDataDocument) { + if (options?.variables?.key === 'arkid15r') { + return { loading: false, data: null, error: new Error('GraphQL error for leader') } + } + return mockUserData(options?.variables?.key) } return { loading: true } })
| ;(useQuery as unknown as jest.Mock).mockImplementation((query, options) => { | ||
| const key = options?.variables?.key | ||
|
|
||
| if (query === GetProjectMetadataDocument) { | ||
| if (key === 'nest') { | ||
| return mockProjectData | ||
| } | ||
| } else if (query === GetTopContributorsDocument) { | ||
| if (key === 'nest') { | ||
| return mockTopContributorsData | ||
| } | ||
| } else if (query === GetLeaderDataDocument) { | ||
| return mockUserData(key) | ||
| } | ||
|
|
||
| return { loading: true } | ||
| }) | ||
| mockRouter = { push: jest.fn() } | ||
| ;(useRouter as jest.Mock).mockReturnValue(mockRouter) | ||
| ; (useRouter as jest.Mock).mockReturnValue(mockRouter) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "About" --full-path --glob '**/*.tsx' --type f | grep -E "(test|page)" | head -20Repository: OWASP/Nest
Length of output: 149
🏁 Script executed:
fd "About" --type f | head -20Repository: OWASP/Nest
Length of output: 288
🏁 Script executed:
# First, let's look at the imports and the mock section in About.test.tsx
cat -n frontend/__tests__/unit/pages/About.test.tsx | head -100Repository: OWASP/Nest
Length of output: 4082
🏁 Script executed:
# Continue reading to find the mock setup around lines 203-221
cat -n frontend/__tests__/unit/pages/About.test.tsx | sed -n '100,230p'Repository: OWASP/Nest
Length of output: 4225
🏁 Script executed:
# Check if there are more imports in the test file or if those constants are defined
cat -n frontend/__tests__/unit/pages/About.test.tsx | grep -E "(import|GetProjectMetadata|GetTopContributor|GetLeaderData|mockProjectData|mockTopContributorsData|mockUserData)"Repository: OWASP/Nest
Length of output: 1883
🏁 Script executed:
# Check if these constants/functions are defined anywhere in the About.test.tsx file
rg -n "GetProjectMetadataDocument|GetTopContributorsDocument|GetLeaderDataDocument|mockProjectData|mockTopContributorsData|mockUserData" frontend/__tests__/unit/pages/About.test.tsx | grep -E "(const|import|function|export|=\s*)"Repository: OWASP/Nest
Length of output: 847
🏁 Script executed:
# Look at the entire file to see if these are defined
wc -l frontend/__tests__/unit/pages/About.test.tsxRepository: OWASP/Nest
Length of output: 102
🏁 Script executed:
# Read the entire file to find where these constants are defined
cat -n frontend/__tests__/unit/pages/About.test.tsx | tail -n +1 | head -n 400Repository: OWASP/Nest
Length of output: 15954
🏁 Script executed:
# Let's check the About page component to see what queries it actually uses
cat -n frontend/src/app/about/page.tsx | head -50Repository: OWASP/Nest
Length of output: 1999
🏁 Script executed:
# Continue reading to see how GetAboutPageDataDocument is used
cat -n frontend/src/app/about/page.tsx | sed -n '50,150p'Repository: OWASP/Nest
Length of output: 3967
🏁 Script executed:
# Check what's actually in the aboutQueries.generated file
cat -n frontend/src/types/__generated__/aboutQueries.generated.ts | head -100Repository: OWASP/Nest
Length of output: 8926
🏁 Script executed:
# Check if these constants are defined anywhere in the test file or imported
rg -n "const mockProjectData|const mockTopContributorsData|const mockUserData|const GetProjectMetadataDocument|const GetTopContributorsDocument|const GetLeaderDataDocument" frontend/__tests__/unit/pages/About.test.tsxRepository: OWASP/Nest
Length of output: 36
🏁 Script executed:
# Let's read more of the test file to see if there are any other beforeEach or mock setup blocks
cat -n frontend/__tests__/unit/pages/About.test.tsx | sed -n '400,600p'Repository: OWASP/Nest
Length of output: 8418
🏁 Script executed:
# Search for where mockProjectData, mockTopContributorsData, and mockUserData are defined
rg -A 5 "mockProjectData\s*=" frontend/__tests__/unit/pages/About.test.tsx | head -30Repository: OWASP/Nest
Length of output: 36
🏁 Script executed:
# Search the entire test directory for where these mock data objects are defined
rg "mockProjectData|mockTopContributorsData|mockUserData" frontend/__tests__ --type=ts --type=tsx | head -30Repository: OWASP/Nest
Length of output: 81
🏁 Script executed:
# Search for where these mock data objects are defined
rg "mockProjectData|mockTopContributorsData|mockUserData" frontend/__tests__ | head -30Repository: OWASP/Nest
Length of output: 2687
Fix undefined mock references in beforeEach block — GetProjectMetadataDocument, GetTopContributorsDocument, GetLeaderDataDocument, mockProjectData, mockTopContributorsData, and mockUserData are not imported or defined.
Lines 206–215 reference constants and functions that don't exist in this file's scope, causing ReferenceError at runtime. These belong to other pages (Projects, Users). The About page imports and uses only GetAboutPageDataDocument (line 9), which is not handled by the mock. Replace the entire mock implementation to match the About page's actual query — it calls useQuery(GetAboutPageDataDocument) with variables including projectKey, excludedUsernames, and three leader usernames. Use mockAboutData (already imported on line 3) as the response. Additionally, line 221 has malformed whitespace (leading ; before (useRouter...)).
🤖 Prompt for AI Agents
In `@frontend/__tests__/unit/pages/About.test.tsx` around lines 203 - 221, The
beforeEach mock for useQuery references undefined symbols from other pages;
replace that mockImplementation to only handle
useQuery(GetAboutPageDataDocument) by reading variables.projectKey (and other
variables like excludedUsernames and leader usernames if present) and return the
imported mockAboutData, remove any branches for
GetProjectMetadataDocument/GetTopContributorsDocument/GetLeaderDataDocument and
references to mockProjectData/mockTopContributorsData/mockUserData, and fix the
malformed whitespace before the useRouter mock by ensuring mockRouter is created
(mockRouter = { push: jest.fn() }) and then calling (useRouter as
jest.Mock).mockReturnValue(mockRouter) without the stray leading semicolon.
| test('renders ErrorDisplay when project is null', async () => { | ||
| ;(useQuery as unknown as jest.Mock).mockImplementation((query) => { | ||
| if (query === GetAboutPageDataDocument) { | ||
| return { | ||
| loading: false, | ||
| data: { | ||
| project: null, | ||
| topContributors: mockAboutData.topContributors, | ||
| leader1: mockAboutData.users['arkid15r'], | ||
| leader2: mockAboutData.users['kasya'], | ||
| leader3: mockAboutData.users['mamicidal'], | ||
| }, | ||
| error: null, | ||
| } | ||
| ;(useQuery as unknown as jest.Mock).mockImplementation((query, options) => { | ||
| if (options?.variables?.key === 'nest') { | ||
| return { loading: false, data: { project: null }, error: null } | ||
| } | ||
| return { | ||
| loading: false, | ||
| data: { user: { avatarUrl: '', company: '', name: 'Dummy', location: '' } }, | ||
| error: null, | ||
| } | ||
| return { loading: true } | ||
| }) |
There was a problem hiding this comment.
Unreachable code on line 570.
The else branch (lines 565–569) unconditionally returns, so return { loading: true } on line 570 is dead code and will never execute. This looks like a leftover from a copy-paste merge.
🧹 Remove the dead return
return { loading: false, data: { project: null }, error: null }
- }
- return {
+ } else {
+ return {
loading: false,
data: { user: { avatarUrl: '', company: '', name: 'Dummy', location: '' } },
error: null,
}
- return { loading: true }
+ }
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test('renders ErrorDisplay when project is null', async () => { | |
| ;(useQuery as unknown as jest.Mock).mockImplementation((query) => { | |
| if (query === GetAboutPageDataDocument) { | |
| return { | |
| loading: false, | |
| data: { | |
| project: null, | |
| topContributors: mockAboutData.topContributors, | |
| leader1: mockAboutData.users['arkid15r'], | |
| leader2: mockAboutData.users['kasya'], | |
| leader3: mockAboutData.users['mamicidal'], | |
| }, | |
| error: null, | |
| } | |
| ;(useQuery as unknown as jest.Mock).mockImplementation((query, options) => { | |
| if (options?.variables?.key === 'nest') { | |
| return { loading: false, data: { project: null }, error: null } | |
| } | |
| return { | |
| loading: false, | |
| data: { user: { avatarUrl: '', company: '', name: 'Dummy', location: '' } }, | |
| error: null, | |
| } | |
| return { loading: true } | |
| }) | |
| test('renders ErrorDisplay when project is null', async () => { | |
| ;(useQuery as unknown as jest.Mock).mockImplementation((query, options) => { | |
| if (options?.variables?.key === 'nest') { | |
| return { loading: false, data: { project: null }, error: null } | |
| } else { | |
| return { | |
| loading: false, | |
| data: { user: { avatarUrl: '', company: '', name: 'Dummy', location: '' } }, | |
| error: null, | |
| } | |
| } | |
| }) |
🧰 Tools
🪛 Biome (2.3.13)
[error] 570-570: This code will never be reached ...
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
🤖 Prompt for AI Agents
In `@frontend/__tests__/unit/pages/About.test.tsx` around lines 560 - 571, The
mock implementation for useQuery inside the test 'renders ErrorDisplay when
project is null' contains an unreachable final "return { loading: true }"
because the prior branches already return unconditionally; remove that dead
return (or restructure the mockImplementation to use if/else if/else and only
return once) so that useQuery mock only returns the intended objects; reference
the test name and the useQuery mockImplementation to locate and fix the dead
code.
| test('renders mission and who its for sections correctly', async () => { | ||
| await act(async () => { | ||
| render(<About />) | ||
| }) | ||
|
|
||
| const missionSection = screen.getByText('Our Mission').closest('div') | ||
| expect(missionSection).toBeInTheDocument() | ||
| expect(screen.getByText('Test mission statement')).toBeInTheDocument() | ||
|
|
||
| const whoItsForSection = screen.getByText("Who It's For").closest('div') | ||
| expect(whoItsForSection).toBeInTheDocument() | ||
| expect(screen.getByText('Test target audience description')).toBeInTheDocument() | ||
| }) | ||
|
|
||
| test('renders mission section', async () => { | ||
| await act(async () => { | ||
| render(<About />) | ||
| }) | ||
| expect(screen.getByText('Our Mission')).toBeInTheDocument() | ||
| expect(screen.getByText('Test mission statement')).toBeInTheDocument() | ||
| }) | ||
|
|
||
| test("renders 'Who It's For' section", async () => { | ||
| await act(async () => { | ||
| render(<About />) | ||
| }) | ||
| expect(screen.getByText("Who It's For")).toBeInTheDocument() | ||
| expect(screen.getByText(/Test target audience description/)).toBeInTheDocument() | ||
| }) | ||
|
|
||
| test('renders key features section', async () => { | ||
| await act(async () => { | ||
| render(<About />) | ||
| }) | ||
| expect(screen.getByText('Key Features')).toBeInTheDocument() | ||
| expect(screen.getByText('Feature 1')).toBeInTheDocument() | ||
| expect(screen.getByText('Feature 2')).toBeInTheDocument() | ||
| expect(screen.getByText('Feature 1 description')).toBeInTheDocument() | ||
| expect(screen.getByText('Feature 2 description')).toBeInTheDocument() | ||
| }) | ||
|
|
||
| test('renders get involved section', async () => { | ||
| await act(async () => { | ||
| render(<About />) | ||
| }) | ||
| expect(screen.getByText('Get Involved')).toBeInTheDocument() | ||
| expect(screen.getByText('Get involved description')).toBeInTheDocument() | ||
| expect(screen.getByText('Way 1')).toBeInTheDocument() | ||
| expect(screen.getByText('Way 2')).toBeInTheDocument() | ||
| expect(screen.getByText('Test call to action')).toBeInTheDocument() | ||
| }) | ||
|
|
||
| test('renders project history timeline section', async () => { | ||
| await act(async () => { | ||
| render(<About />) | ||
| }) | ||
| expect(screen.getByText('Project Timeline')).toBeInTheDocument() | ||
| expect(screen.getByText('Timeline Event 7')).toBeInTheDocument() | ||
| expect(screen.getByText('Timeline Event 2')).toBeInTheDocument() | ||
| expect(screen.queryByText('Timeline Event 1')).not.toBeInTheDocument() | ||
| }) |
There was a problem hiding this comment.
Duplicate test cases — six tests are repeated verbatim.
The following tests at lines 635–695 duplicate earlier tests in the same describe block:
| Duplicate (line) | Original (line) | Test name |
|---|---|---|
| 635 | 228 | renders mission and who its for sections correctly |
| 649 | 242 | renders mission section |
| 657 | 250 | renders 'Who It's For' section |
| 665 | 258 | renders key features section correctly / renders key features section |
| 676 | 272 | renders get involved section correctly / renders get involved section |
| 687 | 286 | renders project history timeline correctly / renders project history timeline section |
Jest will run both copies (last one wins for name collisions), which wastes CI time, produces confusing output, and masks whether the "real" test is passing. Remove the duplicates.
🤖 Prompt for AI Agents
In `@frontend/__tests__/unit/pages/About.test.tsx` around lines 635 - 695, Remove
the duplicated Jest tests in the About.test.tsx file by deleting the repeated
test blocks with these names: "renders mission and who its for sections
correctly", "renders mission section", "renders 'Who It's For' section",
"renders key features section" (or "renders key features section correctly"),
"renders get involved section" (or "renders get involved section correctly"),
and "renders project history timeline section" (or "renders project history
timeline correctly"); keep only the original instances of these tests (the first
occurrences) so each assertion set (render(<About />) + screen.getByText(...)
expectations) appears exactly once.
frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx
Outdated
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/app/board/`[year]/candidates/page.tsx:
- Line 341: The dark-mode hover color for the LinkedIn link is identical to its
base dark color, so update the class on the LinkedIn anchor (the element with
className="text-blue-600 hover:text-blue-700 dark:text-blue-300
dark:hover:text-blue-300") to use a visibly different dark-mode hover shade (for
example change dark:hover:text-blue-300 to dark:hover:text-blue-200 or
dark:hover:text-blue-100) so the link shows a clear hover affordance in dark
theme.
🧹 Nitpick comments (1)
frontend/src/app/board/[year]/candidates/page.tsx (1)
424-453: Metric label color (text-gray-500) is borderline AA and inconsistent with sibling icon color.The icons were changed to
text-gray-800(≈15:1 contrast on white — excellent), but the labels ("Commits", "PRs", "Issues", "Total") usetext-gray-500which yields only ≈4.85:1 on the white card background. While this technically passes AA's 4.5:1 threshold for normal text, it's the weakest contrast in the entire file and inconsistent with the intent of this PR. Usingtext-gray-700ortext-gray-800would align with the other improvements and give comfortable headroom.🎨 Proposed fix (example for one; apply to all four)
- <p className="text-xs text-gray-500 dark:text-gray-200">Commits</p> + <p className="text-xs text-gray-700 dark:text-gray-200">Commits</p>Apply the same change on Lines 435, 444, and 453.
| ) | ||
| }} | ||
| className="text-blue-600 hover:text-blue-700 dark:text-blue-400 dark:hover:text-blue-300" | ||
| className="text-blue-600 hover:text-blue-700 dark:text-blue-300 dark:hover:text-blue-300" |
There was a problem hiding this comment.
Dark-mode hover state on LinkedIn link has no visual change.
dark:text-blue-300 and dark:hover:text-blue-300 resolve to the same color, so hovering provides no visual feedback in dark mode. This reduces the interactive affordance of the link. Use a lighter or different shade for the hover state.
🔧 Proposed fix
- className="text-blue-600 hover:text-blue-700 dark:text-blue-300 dark:hover:text-blue-300"
+ className="text-blue-600 hover:text-blue-700 dark:text-blue-300 dark:hover:text-blue-200"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className="text-blue-600 hover:text-blue-700 dark:text-blue-300 dark:hover:text-blue-300" | |
| className="text-blue-600 hover:text-blue-700 dark:text-blue-300 dark:hover:text-blue-200" |
🤖 Prompt for AI Agents
In `@frontend/src/app/board/`[year]/candidates/page.tsx at line 341, The dark-mode
hover color for the LinkedIn link is identical to its base dark color, so update
the class on the LinkedIn anchor (the element with className="text-blue-600
hover:text-blue-700 dark:text-blue-300 dark:hover:text-blue-300") to use a
visibly different dark-mode hover shade (for example change
dark:hover:text-blue-300 to dark:hover:text-blue-200 or
dark:hover:text-blue-100) so the link shows a clear hover affordance in dark
theme.
3 participants
Description
This PR fixes accessibility issues related to color contrast across the application, addressing failures in WCAG AA standards.
Related Issue(s)
Closes #3797
Checklist