Skip to content

Conversation

@vdimarco
Copy link
Contributor

@vdimarco vdimarco commented Jan 29, 2026

Summary

  • Fixed ESLint jsx-a11y/alt-text warning in app-footer.test.tsx
  • Updated Next Image mock to properly provide alt attribute

Problem

ESLint was reporting a warning:

./src/components/layout/__tests__/app-footer.test.tsx
22:5  Warning: img elements must have an alt prop, either with meaningful text, or an empty string for decorative images.  jsx-a11y/alt-text

Changes

  • Modified the Next Image mock to:
    • Accept optional alt prop in type definition
    • Provide alt={props.alt || ''} to the img element to satisfy accessibility requirements
    • Falls back to empty string when alt is not provided

Testing

  • ✅ All existing tests pass (13/13 passed)
  • ✅ ESLint reports no warnings or errors
  • ✅ TypeScript compilation passes

Verification

pnpm lint  # ✔ No ESLint warnings or errors
pnpm test -- src/components/layout/__tests__/app-footer.test.tsx  # All 13 tests pass

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com

📎 Task: https://www.terragonlabs.com/task/4d3ce2b1-0def-4eaa-8114-64a559aecd2f

Summary by CodeRabbit

Release Notes

  • Tests
    • Updated image component testing to handle optional alt attributes and ensure alt text is consistently rendered in image elements.

✏️ Tip: You can customize this high-level summary in your review settings.

Greptile Overview

Greptile Summary

Fixed ESLint jsx-a11y/alt-text warning in the app-footer.test.tsx test file by updating the Next Image mock.

Changes:

  • Made alt prop optional in the mock's type definition (alt?: string)
  • Added explicit alt={props.alt || ''} to the rendered img element to ensure the alt attribute is always present
  • Falls back to empty string when alt is not provided, satisfying accessibility requirements

Impact:
This is a test-only change that eliminates ESLint warnings without affecting runtime behavior. The mock now correctly mirrors Next.js Image component behavior where alt can be optional but the underlying img element always receives an alt attribute.

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • This is a minimal, focused change that only affects test mocks. It properly fixes an ESLint accessibility warning by making the mock more accurately represent Next.js Image behavior. All tests pass, no runtime code is affected, and the change follows best practices for accessibility.
  • No files require special attention

Important Files Changed

Filename Overview
src/components/layout/tests/app-footer.test.tsx Fixed ESLint accessibility warning by making alt prop optional in Next Image mock with proper fallback

Sequence Diagram

sequenceDiagram
    participant Test as Test Suite
    participant Mock as Next Image Mock
    participant ESLint as ESLint Checker
    
    Note over Test,ESLint: Before Fix
    Test->>Mock: Render Image (no alt specified)
    Mock->>Mock: Spread props to img
    Mock-->>Test: Renders img without alt
    ESLint->>Mock: Check img element
    ESLint-->>Test: ❌ Warning: missing alt prop
    
    Note over Test,ESLint: After Fix
    Test->>Mock: Render Image (no alt specified)
    Mock->>Mock: Check props.alt
    Mock->>Mock: Use props.alt || '' fallback
    Mock-->>Test: Renders img with alt=""
    ESLint->>Mock: Check img element
    ESLint-->>Test: ✅ No warning
Loading

- Fix ESLint jsx-a11y/alt-text warning
- Ensure Image mock provides alt attribute (empty string fallback)
- All tests passing
@vercel
Copy link

vercel bot commented Jan 29, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
gatewayz-frontend Error Error Jan 29, 2026 2:15pm

@chatgpt-codex-connector
Copy link

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai
Copy link

coderabbitai bot commented Jan 29, 2026

📝 Walkthrough

Walkthrough

The test file's mocked next/image component was updated to make the alt prop optional while ensuring the rendered img element always receives an explicit alt attribute, defaulting to an empty string when not provided.

Changes

Cohort / File(s) Summary
Test Mock Update
src/components/layout/__tests__/app-footer.test.tsx
Modified the next/image mock to treat alt as an optional prop (alt?: string) while guaranteeing the rendered img tag always has an explicit alt attribute via alt={props.alt || ''}.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A mock now bends to optional ways,
With alt attributes that always stay,
Empty strings when none appear,
The footer image, bright and clear! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: making the alt prop optional in the Next Image mock to fix a test-related ESLint warning in app-footer.test.tsx.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch terragon/fix-frontend-errors-d0pny4

Comment @coderabbitai help to get the list of available commands and usage tips.

@vdimarco vdimarco changed the title fix(test): add alt prop to Next Image mock in app-footer test fix(test): make Next Image mock alt prop optional in app-footer test Jan 29, 2026
@openhands-ai
Copy link

openhands-ai bot commented Jan 29, 2026

Looks like there are a few issues preventing this PR from being merged!

  • GitHub Actions are failing:
    • CI

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #936 at branch `terragon/fix-frontend-errors-d0pny4`

Feel free to include any additional details that might help me get this PR into a better state.

You can manage your notification settings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants