Uh oh!
There was an error while loading. Please reload this page.
test: configure lint-css to check a11y and rtl - #1203
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughReplaces the RTL-only CSS linting with a unified UnoCSS check that includes both RTL and accessibility presets. Adds a new preset module (uno-preset-a11y.ts) that detects hardcoded font-size tokens (px and em), emits deduplicated warnings or routes them to a provided checker, and suggests existing text-size utilities. Introduces scripts/unocss-checker.ts to run checks per-file, updates package.json scripts and lint-staged, includes presetA11y in uno.config.ts (non‑CI), adds unit tests for the a11y preset, and makes a small class change in LicenseDisplay.vue. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
scripts/unocss-checker.ts (1)
38-48: Consider extracting the duplicated callback logic.The A11y warning handler (lines 38-48) is nearly identical to the RTL handler (lines 27-37), differing only in the label
[A11y]vs[RTL]. Extracting a helper function would reduce duplication.♻️ Suggested refactor
+function createWarningHandler(+ label: string,+ warnings: Map<number, string[]>,+ getContext: () => { idx: number; line: string; filename: string },+): (warning: string, rule: string) => void {+ return (warning, rule) => {+ const { idx, line, filename } = getContext()+ let entry = warnings.get(idx)+ if (!entry) {+ entry = []+ warnings.set(idx, entry)+ }+ const ruleIdx = line.indexOf(rule)+ entry.push(+ `${COLORS.red} ❌ [${label}] ${filename}:${idx}${ruleIdx > -1 ? `:${ruleIdx + 1}` : ''} - ${warning}${COLORS.reset}`,+ )+ }+}Then use it:
constcontext={idx: -1,line: '', filename }constuno=awaitcreateGenerator({presets: [presetWind4(),presetRtl(createWarningHandler('RTL',warnings,()=>context)),presetA11y(createWarningHandler('A11y',warnings,()=>context)),],})// Update context.idx and context.line in the loopuno-preset-a11y.ts (1)
21-26: Consider documenting the mapping relationship with uno.config.ts.The
textPxToClassmapping corresponds to custom theme sizes inuno.config.ts(lines 46-49). A brief comment linking these would help maintainers keep them in sync.📝 Suggested documentation
+// Maps pixel sizes to custom text classes defined in uno.config.ts theme.text+// Keep in sync with theme configuration const textPxToClass: Record<number, string> = { }
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Nice, I miss this one ❤️ (a11y statement page) |
In a recent issue (#1116), we removed all hardcoded font-size classes, migrated them to our own inner classes, and agreed to use rem everywhere. We also added an
rtl:checkstep, which checks whether classes comply withrtlpatterns. Based on this, I'm adding a check for hardcoded font-size classes and slightly refining the solution to prevent this logic from degradingI also set up lint-staged support (it passes all files as a list in the format
node run-script path-a path-b path-c) and added a script to lint-staged. This should work quickly for most cases and will make debugging easierI renamed the command to
lint:cssbecause it is more familiar to the average developer and formally that's what this script doesCloses#1201