Skip to content

Comments

feat: Add custom size props to modal#4252

Open
amanabiy wants to merge 20 commits intomainfrom
dev-v3-amanabiy-modal
Open

feat: Add custom size props to modal#4252
amanabiy wants to merge 20 commits intomainfrom
dev-v3-amanabiy-modal

Conversation

@amanabiy
Copy link
Member

@amanabiy amanabiy commented Feb 13, 2026

Description

This PR extends the Modal component with custom dimension support by adding width and height props, building on the position prop introduced in the previous PR.

Note: The base branch is dev-v3-amanabiy-modal-top-position to isolate changes from the position prop PR for easier review.

New Features:

  • width prop: Allows specifying custom modal width (minimum 320px). Takes precedence over the size prop when provided.
  • height prop: Enables custom modal height with automatic content scrolling when content exceeds the specified height.

The base is made to dev-v3-amanabiy-modal-top-position so that you can review the main changes only related to this PR, and not the previous prs which adds position prop and different sizes.

Behavior:

  • Both props respect viewport constraints - if specified dimensions exceed available space, the modal uses maximum available space
  • When custom height is set and content overflows, the content is scrollable
  • Minimum width constraint ensures that the minimum width builders can set is equal to the smallest size
  • Minimum height constraint ensures content area is at least 60px for accessibility
  • Proper ARIA attributes (role="region", aria-label) are applied to scrollable content regions for screen reader support

Related links, issue #, if available: gfq0AFhyrah9

How has this been tested?

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov
Copy link

codecov bot commented Feb 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.42%. Comparing base (37685a3) to head (78a1bae).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4252   +/-   ##
=======================================
  Coverage   97.42%   97.42%           
=======================================
  Files         891      891           
  Lines       26098    26124   +26     
  Branches     9435     9455   +20     
=======================================
+ Hits        25426    25452   +26     
  Misses        629      629           
  Partials       43       43           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@amanabiy amanabiy force-pushed the dev-v3-amanabiy-modal branch from e48dd57 to 7d6cc10 Compare February 20, 2026 17:52
@amanabiy amanabiy force-pushed the dev-v3-amanabiy-modal branch from edd7556 to e2d17f6 Compare February 23, 2026 08:56
@amanabiy amanabiy force-pushed the dev-v3-amanabiy-modal branch 2 times, most recently from 0e46e92 to fec3d10 Compare February 23, 2026 10:02
@amanabiy amanabiy force-pushed the dev-v3-amanabiy-modal branch from fec3d10 to 29ed6ca Compare February 23, 2026 10:08
@amanabiy amanabiy changed the base branch from main to dev-v3-amanabiy-modal-new-sizes February 23, 2026 10:09
@amanabiy amanabiy changed the base branch from dev-v3-amanabiy-modal-new-sizes to dev-v3-amanabiy-modal-top-position February 23, 2026 10:09
@amanabiy amanabiy marked this pull request as ready for review February 23, 2026 11:09
@amanabiy amanabiy requested a review from a team as a code owner February 23, 2026 11:09
@amanabiy amanabiy force-pushed the dev-v3-amanabiy-modal branch from 9852f0e to a34f8ce Compare February 23, 2026 11:16
@amanabiy amanabiy changed the base branch from dev-v3-amanabiy-modal-top-position to main February 23, 2026 11:17
/**
* Specifies the height of the modal. When provided, the modal content becomes scrollable if it exceeds the specified height.
* If the specified height exceeds available viewport space, the modal will use the maximum available space.
* The minimum height is constrained to ensure the content area is at least 60px for accessibility.
Copy link
Member

Choose a reason for hiding this comment

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

"constrained" suggests that a maximum is applied to a given value, but here it is a minimum. Can we find a different phrasing? Is this the vertical equivalent of the minimum 320px width mentioned in the width prop above? If so, could we harmonize both descriptions?

@amanabiy amanabiy changed the base branch from main to dev-v3-amanabiy-modal-top-position February 23, 2026 17:49
@amanabiy amanabiy changed the base branch from dev-v3-amanabiy-modal-top-position to main February 23, 2026 18:51
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