chore(ai-tools): skills audit + design tooling#3351
Conversation
|
View your CI Pipeline Execution ↗ for commit 9f1fa02 ☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit 64fed33 ☁️ Nx Cloud last updated this comment at |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## evan-poc-agent-tooling #3351 +/- ##
==========================================================
- Coverage 90.29% 89.91% -0.38%
==========================================================
Files 384 276 -108
Lines 6263 5730 -533
Branches 2046 1929 -117
==========================================================
- Hits 5655 5152 -503
+ Misses 600 570 -30
Partials 8 8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dreamwasp
changed the title
sh0ji
left a comment
sh0ji
left a comment
There was a problem hiding this comment.
very nice!
my only suggestions were around the accessibility stuff, which I'm fine iterating on after merging rather than trying to get perfect here. but I'll leave that up to you.
There was a problem hiding this comment.
how do you feel about not including rules? I was on the fence about this when I opened the PR and just included the rule version as an example.
There was a problem hiding this comment.
yeah i'm down with it - alot of this work was adding intra-skill links where i thought would be necessary so i think it makes sense to refactor this to a general accessibility skill.
| ## Form label association | ||
|
|
||
| Match `htmlFor` on `<FormGroupLabel>` with the `id` on the input. `<FormGroup>` auto-wires `aria-live` for `error` and `description` — do not add redundant live regions manually. | ||
| Match **`htmlFor`** on **`<FormGroupLabel>`** with the **`id`** on the control. Base **`<FormGroup>`** renders live regions for **`error`** and **`description`**; **`GridForm`** and **`ConnectedForm`** add field wiring (**`aria-describedby`**, **`aria-invalid`**, first-error **`aria-live`** behavior) — do not add redundant duplicate regions. Depth: **[`skills/gamut-forms/SKILL.md`](../skills/gamut-forms/SKILL.md)**. |
There was a problem hiding this comment.
did you add this bolding or did the AI?
I've noticed that AI seems to love bolding things, but have wondered how meaningful it is. it certainly would be if it changed the meaning like <em> does but these examples are just marking the words as important by setting them apart visually, which the <code>/` already does.
no need to change anything, especially if you decide to kill this rule. just reminded me that I've been wondering this.
There was a problem hiding this comment.
def AI, i'll change it
| <li>...</li> | ||
| </ul> | ||
| ``` | ||
| Prefer native HTML, minimal ARIA, correct roles, visible names, focus visibility, semantic color / `ColorMode`, and Gamut primitives — see the always-loaded **Gamut Accessibility Rules**: [`accessibility.mdc`](../../rules/accessibility.mdc). This skill adds **Gamut component behavior** and audit detail below. |
There was a problem hiding this comment.
I don't think accessibility makes sense as always-loaded context, which is one of the reasons I don't love rules (the other being that they're Cursor-only). there are lots of times when a developer won't want/need it in context like if they're writing an RFC for a backend system or just using the agent to help them explore a codebase.
I do like the idea of shipping a general accessibility skill—distributing it through the design system makes sense. but maybe there's some way we can break this up into "general accessibility" and "Gamut accessibility" skills?
There was a problem hiding this comment.
agreed - i'll refactor a bit and use the same intra-skill logic to make sure they are loaded when needed
|
|
||
| --- | ||
|
|
||
| ## General rules |
There was a problem hiding this comment.
These 5 "general rules" are the only ones that I wrote myself because it's where I'm most opinionated. If you do think it makes sense for Gamut to ship a "general accessibility" skill, could we discuss these as the baseline?
- Prefer HTML over ARIA
- A Role is a Promise
- ARIA can both cloak and enhance
- Align accessible names with visible copy
- Treat missing visible labels as a design smell
|
📬 Published Alpha Packages:
|
|
🚀 Styleguide deploy preview ready! Preview URL: https://6a0b7f864be7226e18852d28--gamut-preview.netlify.app |
3 participants
Overview
Refactors skills to reflect actual practices and adds new tag for adding themed DESIGN.theme.md. I also removed the figma tool since the figma.config.json is only for the CodeConnect tool
Testing Instructions
yarn add @codecademy/gamut@insert-alpha-here. this will just pull the new agent assets as well as the newgamutbinary command, which you can run withyarn gamutornpx gamut.yarn installto register the newgamutbinary.yarn gamut plugin install cursor|claude.yarn gamut plugin install(without a target) defaults to cursor.yarn gamut plugin install claudeinstalls both the plugin and a custom marketplace.yarn gamut plugin install cursor --theme coreinstalls the tools & proper DESIGN.theme.md fileclaude plugins list. You should seegamut-design-systemlisted.