Skip to content

fix: Prevent description duplication in Modal header#7609

Merged
raunakab merged 3 commits intomainfrom
fix/modal-description-duplication
Jan 21, 2026
Merged

fix: Prevent description duplication in Modal header#7609
raunakab merged 3 commits intomainfrom
fix/modal-description-duplication

Conversation

@raunakab
Copy link
Copy Markdown
Contributor

@raunakab raunakab commented Jan 21, 2026

Description

When no description prop is provided to Modal.Header, the component was falling back to rendering the title as the description, causing visual and ARIA duplication.

Changes:

  • Add hasDescription state to ModalContext to track if description exists
  • Only render DialogPrimitive.Description when the description prop is provided
  • Suppress Radix aria-describedby warning via conditional prop when no description prop is provided

Fixes #7603.

How Has This Been Tested?

No UI changes; screenshots not required.

Additional Options

  • [Optional] Override Linear Check

Summary by cubic

Prevent duplicate descriptions in Modal.Header. We now render a description only when the prop is provided and remove aria-describedby when it’s not, preventing visual/ARIA duplication and suppressing the Radix warning.

Written for commit 9c78588. Summary will update on new commits.

When no description prop is provided to Modal.Header, the component
was falling back to rendering the title as the description, causing
visual and ARIA duplication.

Changes:
- Add hasDescription state to ModalContext to track if description exists
- Only render DialogPrimitive.Description when description prop is provided
- Suppress Radix aria-describedby warning via conditional prop when no description

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@raunakab raunakab requested a review from a team as a code owner January 21, 2026 02:00
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Jan 21, 2026

Greptile Summary

This PR fixes a bug where the Modal component displayed duplicate text when no description prop was provided. Previously, the component fell back to rendering the title as the description ({description || title}), causing both visual and ARIA accessibility duplication.

  • Added hasDescription state to ModalContext to track whether a description prop exists
  • Conditionally renders DialogPrimitive.Description only when description is truthy
  • Suppresses Radix UI's aria-describedby warning by setting the attribute to undefined when no description exists
  • Uses a useEffect in ModalHeader to synchronize the description state with the context

Confidence Score: 5/5

  • This PR is safe to merge - it's a straightforward bug fix with minimal scope and no breaking changes
  • The changes are well-contained to a single file, follow React best practices, maintain backward compatibility (existing callers with description props work identically), and properly address both the visual duplication and accessibility warning issues
  • No files require special attention

Important Files Changed

Filename Overview
web/src/refresh-components/Modal.tsx Added hasDescription state to track description prop presence, conditionally renders DialogPrimitive.Description only when description exists, and suppresses Radix aria-describedby warning when no description provided. Fixes title duplication issue.

Sequence Diagram

sequenceDiagram
    participant User
    participant ModalContent
    participant ModalContext
    participant ModalHeader
    participant DialogPrimitive.Content
    participant DialogPrimitive.Description

    User->>ModalContent: Opens modal
    ModalContent->>ModalContext: Creates context with hasDescription=false
    ModalContent->>ModalHeader: Renders header
    
    alt description prop provided
        ModalHeader->>ModalContext: setHasDescription(true)
        ModalHeader->>DialogPrimitive.Description: Renders description
        ModalContext-->>DialogPrimitive.Content: aria-describedby set normally
    else no description prop
        ModalHeader->>ModalContext: setHasDescription(false)
        Note over ModalHeader: DialogPrimitive.Description not rendered
        ModalContext-->>DialogPrimitive.Content: aria-describedby=undefined
    end
Loading

@raunakab raunakab enabled auto-merge January 21, 2026 02:02
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="web/src/refresh-components/Modal.tsx">

<violation number="1" location="web/src/refresh-components/Modal.tsx:340">
P2: When a description is provided, hasDescription is only set after the first paint via useEffect, so the dialog initially renders without aria-describedby. This can prevent screen readers from announcing the description on open. Use useLayoutEffect (or another pre-paint update) so aria-describedby is set before the dialog is announced.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Copy link
Copy Markdown
Contributor

@evan-onyx evan-onyx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cubic said smth maybe valid

@raunakab raunakab added this pull request to the merge queue Jan 21, 2026
@evan-onyx evan-onyx removed this pull request from the merge queue due to a manual request Jan 21, 2026
@raunakab raunakab enabled auto-merge January 21, 2026 04:01
@raunakab raunakab added this pull request to the merge queue Jan 21, 2026
Merged via the queue into main with commit 15d02f6 Jan 21, 2026
84 of 86 checks passed
@raunakab raunakab deleted the fix/modal-description-duplication branch January 21, 2026 04:37
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