-
-
Notifications
You must be signed in to change notification settings - Fork 432
fix(a11y): add wcag-aa color contrast #1170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 11 commits
da05bd1
19f3835
21bee98
10e4b07
fdd4c99
7c5ee29
dd138c0
10e507c
ed73e25
9048794
8e7ab4a
1a942b0
ff15537
65807a1
6294ef2
ac43111
cda6630
b22c871
c86f728
b246b9d
089b8eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,7 +63,7 @@ const typesHref = computed(() => { | |
| variant="button-secondary" | ||
| size="small" | ||
| :to="typesHref" | ||
| classicon="i-carbon-checkmark" | ||
| classicon="i-carbon:checkmark" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check for old-style icon syntax (i-carbon-name pattern)
echo "=== Old-style i-carbon-* syntax (hyphens) ==="
rg "i-carbon-[a-z]" --type vue -g '!node_modules' | wc -l
echo "Count of files/lines with old syntax:"
rg "i-carbon-[a-z]" --type vue -g '!node_modules' -l | wc -l
echo ""
echo "=== New-style i-carbon:* syntax (colons) ==="
rg "i-carbon:[a-z]" --type vue -g '!node_modules' | wc -l
echo "Count of files/lines with new syntax:"
rg "i-carbon:[a-z]" --type vue -g '!node_modules' -l | wc -l
echo ""
echo "=== Detailed breakdown of new-style syntax ==="
rg "i-carbon:[a-z]+" --type vue -g '!node_modules' -o | sort | uniq -c | sort -rn | head -20
echo ""
echo "=== Detailed breakdown of old-style syntax ==="
rg "i-carbon-[a-z]+" --type vue -g '!node_modules' -o | sort | uniq -c | sort -rn | head -20Repository: npmx-dev/npmx.dev Length of output: 517 🏁 Script executed: #!/bin/bash
# Check for old-style icon syntax (i-carbon-name pattern)
echo "=== Old-style i-carbon-* syntax (hyphens) in .vue files ==="
rg "i-carbon-[a-z]" -g '*.vue' | wc -l
echo ""
echo "=== New-style i-carbon:* syntax (colons) in .vue files ==="
rg "i-carbon:[a-z]" -g '*.vue' | wc -l
echo ""
echo "=== Sample of new-style syntax matches ==="
rg "i-carbon:[a-z]+" -g '*.vue' -o | sort | uniq -c | sort -rn | head -20
echo ""
echo "=== Sample of old-style syntax matches ==="
rg "i-carbon-[a-z]+" -g '*.vue' -o | sort | uniq -c | sort -rn | head -20
echo ""
echo "=== Check the specific file mentioned ==="
if [ -f "app/components/Package/MetricsBadges.vue" ]; then
echo "File exists. Checking line 66:"
sed -n '60,72p' "app/components/Package/MetricsBadges.vue" | cat -n
fiRepository: npmx-dev/npmx.dev Length of output: 3334 Update icon syntax to use colons consistently across all Vue components. The codebase currently has mixed icon syntax: 202 instances of the new colon-based style (
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an small optimization for UnoCSS preset-icons: for single icon name (without dash) the result is the same, but for long icon names using We need to find a way to fix this, it is hard since we can add more and more collections and ppl should use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Here are several approaches to enforce this pattern consistently: 1. ESLint Custom RuleCreate a custom ESLint rule that detects 2. Pre-commit HookAdd a Git hook that scans staged files for the old pattern: # .husky/pre-commit or similar
if git diff --cached --name-only | grep -E '\.(vue|ts|js)$' | xargs grep -E 'i-carbon-[a-z]'; then
echo "❌ Found old icon syntax (i-carbon-name). Please use i-carbon:name instead."
exit 1
fi3. Automated Migration ScriptI can help create a script to fix all remaining 71 instances automatically. Would you like me to generate a script that:
4. DocumentationAdd this to your contributing guidelines or a dedicated icon usage document explaining:
5. CI CheckAdd a CI step that fails if old syntax is detected in new code: - name: Check icon syntax
run: |
if grep -r "i-carbon-[a-z]" app/ --include="*.vue"; then
echo "Old icon syntax detected"
exit 1
fiWould you like me to help implement any of these solutions? I can generate the migration script or create the linting rules for you. /ᐠ。ꞈ。ᐟ ✏️ Learnings added
🧠 Learnings used
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you're killing everyone, there are still a lot of icons using
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Let me take a much more practical approach: Gradual, Optional MigrationInstead of breaking builds or forcing everyone to use bash hooks, here are gentler options: 1. One-time migration script (Node.js/JavaScript)I can create a cross-platform Node.js script that you can run once to migrate all remaining old-syntax icons. This would be:
2. Documentation onlySimply document the preferred syntax in your contributing guide for future reference, without any enforcement. New code naturally adopts it over time. 3. Optional VS Code snippetAdd a VS Code snippet that auto-completes with the colon syntax, making it easier for contributors to use the right pattern without forcing it. Would you like me to create a Node.js migration script that you can review and run when convenient? It would be a simple, non-intrusive way to update the remaining 71 instances without impacting anyone's workflow. Again, sorry for the overly aggressive suggestions! /ᐠ - ˕ -マ 🧠 Learnings used |
||
| > | ||
| {{ $t('package.metrics.types_label') }} | ||
| </LinkBase> | ||
|
|
@@ -73,10 +73,10 @@ const typesHref = computed(() => { | |
| :tabindex="0" | ||
| :classicon=" | ||
| isLoading | ||
| ? 'i-carbon-circle-dash motion-safe:animate-spin' | ||
| ? 'i-carbon:circle-dash motion-safe:animate-spin' | ||
| : hasTypes | ||
| ? 'i-carbon-checkmark' | ||
| : 'i-carbon-close' | ||
| ? 'i-carbon:checkmark' | ||
| : 'i-carbon:close' | ||
| " | ||
| > | ||
| {{ $t('package.metrics.types_label') }} | ||
|
|
@@ -94,10 +94,10 @@ const typesHref = computed(() => { | |
| :variant="hasEsm && !isLoading ? 'default' : 'ghost'" | ||
| :classicon=" | ||
| isLoading | ||
| ? 'i-carbon-circle-dash motion-safe:animate-spin' | ||
| ? 'i-carbon:circle-dash motion-safe:animate-spin' | ||
| : hasEsm | ||
| ? 'i-carbon-checkmark' | ||
| : 'i-carbon-close' | ||
| ? 'i-carbon:checkmark' | ||
| : 'i-carbon:close' | ||
| " | ||
| > | ||
| ESM | ||
|
|
@@ -112,7 +112,7 @@ const typesHref = computed(() => { | |
| tabindex="0" | ||
| :variant="isLoading ? 'ghost' : 'default'" | ||
| :classicon=" | ||
| isLoading ? 'i-carbon-circle-dash motion-safe:animate-spin' : 'i-carbon-checkmark' | ||
| isLoading ? 'i-carbon:circle-dash motion-safe:animate-spin' : 'i-carbon:checkmark' | ||
| " | ||
| > | ||
| CJS | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.