feat: draft UI for prerequisite summary by jloh02 · Pull Request #4401 · nusmodifications/nusmods

jloh02 · 2026-05-20T16:17:52Z

vercel · 2026-05-20T16:17:57Z

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
nusmods-export	Ignored	Preview	May 28, 2026 11:58am
nusmods-website	Ignored	Preview	May 28, 2026 11:58am

codecov · 2026-05-21T12:13:28Z

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 56.49%. Comparing base (988c6fd) to head (718b325).
⚠️ Report is 236 commits behind head on master.

Files with missing lines	Patch %	Lines
website/src/views/modules/ModulePageContent.tsx	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4401      +/-   ##
==========================================
+ Coverage   54.52%   56.49%   +1.97%     
==========================================
  Files         274      318      +44     
  Lines        6076     6970     +894     
  Branches     1455     1684     +229     
==========================================
+ Hits         3313     3938     +625     
- Misses       2763     3032     +269

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jloh02 · 2026-05-21T12:52:01Z

@greptile

greptile-apps · 2026-05-21T12:55:04Z

Greptile Summary

This PR introduces a RequisiteRulePopup component — an info icon that displays the full structured prerequisite rule string in a Tippy tooltip next to the "Prerequisite Tree" section heading. It also refactors tooltip theming by introducing dedicated --tooltip-bg and --tooltip-color CSS variables (separate from the general --gray and --body-bg used before), and updates the module tree's user-facing warning text.

RequisiteRulePopup: Renders an accessible <button> with an Info icon; hides itself when the raw prerequisiteRule string is identical to the human-readable prerequisite summary or empty.
CSS variable refactor: Introduces --tooltip-bg and --tooltip-color with proper light/dark-mode values, decoupling tooltip colours from the general grey palette. The dark-mode tooltip correctly switches to a dark background (#3a3a3a) with body text colour, fixing what was previously an inverted-contrast tooltip.
Wiring: ModulePageContent renders RequisiteRulePopup in a new sectionHeadingRow flex container alongside the <h2>, only when module.prerequisiteRule is set.

Confidence Score: 5/5

Safe to merge — the changes are additive (new component + CSS variable indirection) and do not touch any critical data paths.

The new component correctly gates its own rendering with two early-return guards, is accessible (aria-label, focus-visible outline), and is only surfaced when module.prerequisiteRule is present. The CSS variable refactor cleanly isolates tooltip colours and improves the dark-mode tooltip from an inverted-contrast state to a proper dark-on-dark scheme. No existing behaviour is changed for modules without prerequisiteRule.

No files require special attention.

Sequence Diagram

sequenceDiagram
    participant User
    participant ModulePageContent
    participant RequisiteRulePopup
    participant Tooltip as Tooltip (Tippy)
    participant LinkModuleCodes

    ModulePageContent->>RequisiteRulePopup: "rule=module.prerequisiteRule, summary=module.prerequisite"
    RequisiteRulePopup->>RequisiteRulePopup: rule.trim() empty? → return null
    RequisiteRulePopup->>RequisiteRulePopup: "summary.trim() === rule.trim()? → return null"
    RequisiteRulePopup->>Tooltip: "content=div.content with LinkModuleCodes"
    Tooltip-->>User: renders button with aria-label
    User->>Tooltip: hover / focus / touch-hold on button
    Tooltip-->>User: shows prerequisite rule popup
    Tooltip->>LinkModuleCodes: render rule string with module code hyperlinks

_{Reviews (4): Last reviewed commit: "Merge branch 'master' into feat/support-..." | Re-trigger Greptile}

greptile-apps · 2026-05-21T12:55:08Z

+  &:hover,
+  &:focus {
+    outline: none;
+    color: theme-color();
+  }


Focus outline removed without replacement

outline: none on :focus discards the browser's default focus ring for keyboard users. The icon color change alone (from --gray-light to theme-color()) is unlikely to meet WCAG 2.1 SC 2.4.11's visible focus requirement, and many custom themes may have low contrast between these two values. Consider replacing with a visible focus-visible outline (e.g., outline: 2px solid currentColor) instead of suppressing it.

Prompt To Fix With AI

This is a comment left during a code review. Path: website/src/views/components/module-info/RequisiteRulePopup.scss Line: 19-23 Comment: **Focus outline removed without replacement** `outline: none` on `:focus` discards the browser's default focus ring for keyboard users. The icon color change alone (from `--gray-light` to `theme-color()`) is unlikely to meet WCAG 2.1 SC 2.4.11's visible focus requirement, and many custom themes may have low contrast between these two values. Consider replacing with a visible focus-visible outline (e.g., `outline: 2px solid currentColor`) instead of suppressing it. How can I resolve this? If you propose a fix, please make it concise.

Fixed in 3a74cc1 — split hover/focus styles and added a :focus-visible outline (2px solid currentColor with offset) while keeping outline: none only for :focus:not(:focus-visible).

greptile-apps · 2026-05-21T12:55:29Z

Confidence Score: 4/5

Safe to merge; the new popup is an additive UI element and the CSS variable refactor corrects an existing dark-mode bug rather than introducing new regressions.

The only non-trivial concern is that the trigger button suppresses the browser focus ring entirely without a keyboard-accessible replacement, which could affect users navigating by keyboard. Everything else — the early-return guards, tooltip content rendering, CSS variable scoping, and test coverage — looks correct.

website/src/views/components/module-info/RequisiteRulePopup.scss — the .trigger focus styles warrant a second look for accessibility.

Important Files Changed

Filename	Overview
website/src/views/components/module-info/RequisiteRulePopup.tsx	New component that shows the raw structured prerequisite rule in an interactive Tippy tooltip when it differs from the human-readable summary. Logic is clean; early-return guards handle empty and identical strings correctly.
website/src/views/components/module-info/RequisiteRulePopup.scss	Styles for the popup trigger and content. Focus outline is removed without a visible replacement, which is a keyboard accessibility concern.
website/src/styles/utils/css-variables.scss	Adds --tooltip-bg and --tooltip-color CSS variables for both light and dark modes, fixing a pre-existing dark mode bug where tooltip text was dark-on-dark.
website/src/styles/tippy/tippy.css	Migrates tooltip arrow and background references from --gray / --body-bg to the new --tooltip-bg / --tooltip-color variables, correcting dark-mode rendering.
website/src/views/modules/ModulePageContent.tsx	Wraps the 'Prerequisite Tree' heading in a flex row and conditionally renders the new RequisiteRulePopup when module.prerequisiteRule is present.

Sequence Diagram

sequenceDiagram
    participant User
    participant ModulePageContent
    participant RequisiteRulePopup
    participant Tooltip
    participant LinkModuleCodes

    ModulePageContent->>ModulePageContent: render Prerequisite Tree section
    alt module.prerequisiteRule is set
        ModulePageContent->>RequisiteRulePopup: rule, summary
        alt "rule.trim() is empty OR summary === rule"
            RequisiteRulePopup-->>ModulePageContent: null
        else rule differs from summary
            RequisiteRulePopup-->>ModulePageContent: Info icon button
            User->>RequisiteRulePopup: hover / focus / hold
            RequisiteRulePopup->>Tooltip: show content
            Tooltip->>LinkModuleCodes: render rule string
            LinkModuleCodes-->>User: rule text with clickable module code links
        end
    end

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
website/src/views/components/module-info/RequisiteRulePopup.scss:19-23
**Focus outline removed without replacement**

`outline: none` on `:focus` suppresses the browser's default focus ring for keyboard users without a visible alternative. The color shift to `theme-color()` on a small icon button is unlikely to meet WCAG 2.4.7 Focus Visible. Consider using `outline: none` only on `:focus:not(:focus-visible)` and keeping the ring for `:focus-visible`, or adding an explicit `box-shadow`/`outline` replacement to keep keyboard navigation accessible.

_{Reviews (2): Last reviewed commit: "Merge branch 'master' into feat/support-..." | Re-trigger Greptile}

greptile-apps · 2026-05-21T12:55:33Z

+  &:hover,
+  &:focus {
+    outline: none;
+    color: theme-color();
+  }


Focus outline removed without replacement

outline: none on :focus suppresses the browser's default focus ring for keyboard users without a visible alternative. The color shift to theme-color() on a small icon button is unlikely to meet WCAG 2.4.7 Focus Visible. Consider using outline: none only on :focus:not(:focus-visible) and keeping the ring for :focus-visible, or adding an explicit box-shadow/outline replacement to keep keyboard navigation accessible.

Prompt To Fix With AI

This is a comment left during a code review. Path: website/src/views/components/module-info/RequisiteRulePopup.scss Line: 19-23 Comment: **Focus outline removed without replacement** `outline: none` on `:focus` suppresses the browser's default focus ring for keyboard users without a visible alternative. The color shift to `theme-color()` on a small icon button is unlikely to meet WCAG 2.4.7 Focus Visible. Consider using `outline: none` only on `:focus:not(:focus-visible)` and keeping the ring for `:focus-visible`, or adding an explicit `box-shadow`/`outline` replacement to keep keyboard navigation accessible. How can I resolve this? If you propose a fix, please make it concise.

Same fix as above — addressed in 3a74cc1.

Use :focus-visible with a visible outline so keyboard users can see focus while mouse clicks still suppress the default ring. Co-authored-by: Cursor <cursoragent@cursor.com>

feat: draft UI for prerequisite summary

9c8fa06

fix: ci

b041ec0

Merge branch 'master' into feat/support-new-api-fields

c80a112

jloh02 marked this pull request as ready for review May 21, 2026 12:51

greptile-apps Bot reviewed May 21, 2026

View reviewed changes

jloh02 and others added 3 commits May 21, 2026 23:04

fix: restore keyboard focus indicator on requisite rule popup

3a74cc1

Use :focus-visible with a visible outline so keyboard users can see focus while mouse clicks still suppress the default ring. Co-authored-by: Cursor <cursoragent@cursor.com>

Merge branch 'master' into feat/support-new-api-fields

6ae0745

Merge branch 'master' into feat/support-new-api-fields

718b325

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: draft UI for prerequisite summary#4401

feat: draft UI for prerequisite summary#4401
jloh02 wants to merge 6 commits into
masterfrom
feat/support-new-api-fields

jloh02 commented May 20, 2026 •

edited

Loading

Uh oh!

vercel Bot commented May 20, 2026 •

edited

Loading

Uh oh!

codecov Bot commented May 21, 2026 •

edited

Loading

Uh oh!

jloh02 commented May 21, 2026

Uh oh!

greptile-apps Bot commented May 21, 2026 •

edited

Loading

Greptile Summary

Confidence Score: 5/5

Sequence Diagram

Uh oh!

greptile-apps Bot May 21, 2026

Uh oh!

jloh02 May 21, 2026

Uh oh!

greptile-apps Bot commented May 21, 2026

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps Bot May 21, 2026

Uh oh!

jloh02 May 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

jloh02 commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jloh02 commented May 21, 2026

Uh oh!

greptile-apps Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Sequence Diagram

Uh oh!

greptile-apps Bot May 21, 2026

Choose a reason for hiding this comment

Uh oh!

jloh02 May 21, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented May 21, 2026

Confidence Score: 4/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps Bot May 21, 2026

Choose a reason for hiding this comment

Uh oh!

jloh02 May 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jloh02 commented May 20, 2026 •

edited

Loading

vercel Bot commented May 20, 2026 •

edited

Loading

codecov Bot commented May 21, 2026 •

edited

Loading

greptile-apps Bot commented May 21, 2026 •

edited

Loading