Map lesson materials icon chips onto CADS wash tokens; fix checkbox box sizing - #74640
Map lesson materials icon chips onto CADS wash tokens; fix checkbox box sizing#74640levadadenys wants to merge 2 commits into
Conversation
The six resource-type icon chips styled themselves from raw legacy primitives (--sentiment-warning-20/-60, --brand-aqua-20/-70, --accent-strawberry-10/-50, ...) plus one legacy semantic (--background-brand-purple-extra-light). Under codeai-next those primitives alias step-for-step into the CADS ramp, so a -20 fill is now a saturated mid-ramp value rather than a wash and the -60 glyph lands almost on top of it: 1.52:1 for the slides chip. Each chip now takes the CADS `light` background step -- the single wash step that replaced the legacy light/extra-light pair -- with the `secondary` text step for the glyph, the pairing tags.module.scss already uses. Semantic names also invert on their own under [data-theme='Dark'], which the primitives did not; before this, the pink "external link" chip was pink-on-pink in Dark. Contrast, from the token values, Light / Dark: slides 5.35:1 / 8.04:1 video 8.29:1 / 11.08:1 lesson plan, custom 10.58:1 / 11.33:1 link 7.41:1 / 8.18:1 files 7.51:1 / 9.44:1 The legacy aqua, teal and purple families collapse into the one CADS brand family, so the custom-resource chip no longer has a second brand hue and shares the lesson-plan color. The glyph and the label still distinguish them. Also carries an unrelated fast-follow to the checkbox box model: dashboard imports Bootstrap 3, whose reset applies box-sizing: border-box to *:before, so the checkbox's i::before shrank its declared box to a smaller inner box behind the 2px border. Storybook, which does not load dashboard's application.scss, was unaffected -- hence the size only being wrong on dashboard. Declaring content-box on the pseudo-element restores the size the sizing rules intend, matching the declaration already on the i element itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a648e6372
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🖼️ Storybook Visual Comparison Report✅ No Storybook eyes differences detected! |
apps/src/AGENTS.md: lead with the one load-bearing sentence and stop; mechanism narratives and design rationale belong in the PR description. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
moshebaricdo
left a comment
There was a problem hiding this comment.
Ty, lgtm w/ exception of something odd happening with colors.
| background-color: var(--sentiment-warning-20); | ||
| color: var(--sentiment-warning-60); | ||
| background-color: var(--background-warning-light); | ||
| color: var(--text-warning-secondary); |
There was a problem hiding this comment.
These colors in the screenshot caught my attention. They seem to use the right CADS colors (which all seem correct in this file) but are producing a different looking result. I spot checked the other colors and got similar differences.
(The black box is the end-result we want using text-x-primary on background-x-light & unrelated to colors but can we make the icon for slides the solid fill version to match the others)
Two rebrand fast follows: the [Lesson materials] resource-type icon chips, and the checkbox box model on dashboard.
Before:



After:



Lesson materials icon chips
The six chips styled themselves from raw legacy primitives —
--sentiment-warning-20/-60,--brand-aqua-20/-70,--accent-strawberry-10/-50— plus one legacy semantic,--background-brand-purple-extra-light. Undercodeai-nextthose primitives alias step-for-step into the CADS ramp, so a-20fill is now a saturated mid-ramp value rather than a wash, and the-60glyph lands almost on top of it: 1.52:1 for the slides chip. That is the invisible presentation icon in Moshe's report.Each chip now takes the CADS
lightbackground step — the single wash step that replaced the legacy light/extra-light pair — with thesecondarytext step for the glyph. That is the pairingtags.module.scssalready uses for a glyph on a brand wash. Semantic names also invert on their own under[data-theme='Dark'], which the raw primitives did not: before this, the pink "external link" chip was pink-on-pink in Dark.Contrast computed from the token values, all past the 3:1 floor for non-text graphics:
One thing for the reviewer. The legacy aqua, teal and purple families collapse into the one CADS brand family, so the custom-resource chip has no second brand hue left and now shares the lesson-plan color. The glyph (
shapesvsfile-lines) and the label still distinguish them. If design wants them visually separate,accent-orangeis the only CADS family these six do not already use — say so and I'll switch it.Checkbox box model
Separate fast follow, in
component-library/src/checkbox/checkbox.module.scss: one line,box-sizing: content-boxon thei::beforethat draws the box.Dashboard imports Bootstrap 3, whose reset applies
box-sizing: border-boxto*:before. The pseudo-element therefore shrank its declared box to a smaller inner box behind the 2px border, so the checkbox rendered at the wrong size — and only on dashboard, since Storybook does not load dashboard'sapplication.scss. Declaringcontent-boxrestores the size the per-size rules intend, and matches the declaration already on theielement itself four lines above.Links
Testing story
primitiveColors.css+colors.css+brandOverrides.css) underdata-brand='codeai-next', in Light and Dark, and computed every contrast ratio from the resolved values — table above.apps/test/unit/templates/teacherNavigation/lessonMaterials/— 39 tests pass.appsbuild plus a signed-in teacher with a unit assigned; the checkbox change is dashboard-only by nature, so it needs the branch build to see at all. Both worth a look there.Deployment notes
Standard merge-and-deploy. No token definitions change — the chip work is a call-site migration from legacy primitive names to CADS semantic names, which resolve under both brands via the token bridge.