Conversation
pganesh-apphelix
force-pushed
the
fix/TNL2-551
branch
from
397c2cd to
e29c833
Compare
There was a problem hiding this comment.
Pull request overview
Updates the Flashcards and Matching game styles to increase visual contrast, aiming to improve accessibility of headings, buttons, and interactive card/choice elements.
Changes:
- Replaces several theme-token color usages with higher-contrast hex colors for titles/body text.
- Updates primary button backgrounds/hover colors to a higher-contrast blue.
- Adjusts Matching “box” borders/backgrounds across default/hover/selected/matched/incorrect states for stronger differentiation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
games/static/css/matching.css |
Adjusts text colors and interactive “box” state colors/borders for improved contrast. |
games/static/css/flashcards.css |
Updates text colors, button colors/hover states, and help popover styling for improved contrast. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .flashcard-left-button:hover:not(:disabled), | ||
| .flashcard-right-button:hover:not(:disabled) { | ||
| background: var(--light-400, #E1DDDB); | ||
| background: #1D4ED8; | ||
| transform: scale(1.03); | ||
| } |
There was a problem hiding this comment.
The hover state for the flashcard navigation buttons sets a dark blue background, but the arrow SVGs in games/static/html/flashcards.html use a hard-coded fill="#00262B", so the icon color does not change on hover. This results in insufficient contrast (and a regression from the prior light hover background). Consider styling the SVG/path fill on hover (and ideally :focus-visible) or switching the SVG to fill="currentColor" and setting the button color accordingly.
| .flashcard-help-icon svg { | ||
| color: var(--primary-500, #00262B); | ||
| color: #1D4ED8; | ||
| } |
There was a problem hiding this comment.
.flashcard-help-icon svg { color: ... } will not affect the help icon as currently authored: the <path> in games/static/html/flashcards.html has an explicit fill attribute. To actually change the icon color, target fill via CSS (e.g., svg path) or update the SVG markup to use fill="currentColor" and control it via color.
|
|
||
| .flashcards-title { | ||
| color: var(--primary-500, #00262B); | ||
| color: #111827; |
There was a problem hiding this comment.
These rules replace themeable var(--primary-500, ...) color tokens with hard-coded hex values. Since the rest of the file still relies on CSS variables with fallbacks (e.g., var(--white, ...), var(--light-200, ...)), this makes it harder for the host platform/theme to override colors. Consider keeping var(...) usage (or defining a local custom property) and only changing the fallback to the higher-contrast value.
| color: #111827; | |
| color: var(--gray-900, #111827); |
|
|
||
| .matching-start-title { | ||
| color: var(--primary-500, #00262B); | ||
| color: #111827; |
There was a problem hiding this comment.
These text colors were changed from themeable var(--primary-500, ...) tokens to hard-coded hex values. Since this stylesheet still uses CSS variables with fallbacks elsewhere, this reduces theming/override flexibility. Consider retaining var(...) (or introducing a local custom property) and updating only the fallback to the desired higher-contrast color.
| color: #111827; | |
| color: var(--primary-500, #111827); |
| padding: 16px; | ||
| overflow: auto; | ||
| border-radius: 8px; | ||
| border: 2px solid var(--light-300, #F2F0EF); | ||
| background: var(--white, #FFFFFF); | ||
| border: 2px solid #374151; | ||
| background: #F9FAFB; | ||
| cursor: pointer; | ||
| transition: all 0.2s ease; | ||
| text-align: center; | ||
| } |
There was a problem hiding this comment.
.matching-box is styled as an interactive control (cursor + hover states), but in games/static/html/matching.html it is a clickable <div> (not keyboard focusable by default). For accessibility, consider switching it to a <button> or adding tabindex="0" + appropriate ARIA/keyboard handlers, and add a visible :focus-visible style to match the hover/selected states.
No description provided.