Skip to content

Conversation

@Mayank-Tripathi32
Copy link
Contributor

What?

Closes #70611

Adds CSS class support to modal children container when using fill size or fullscreen mode to enable proper height styling.

Why?

Currently, when using the Modal component with size="fill" or isFullScreen={true}, the children container div doesn't have proper height styling. This makes it difficult for developers to create content that takes up the full height of the modal, as the container collapses to its content height rather than expanding to fill the available space.

The issue affects developers who want to create modals with content that needs to utilize the full available height, such as forms with bottom-positioned buttons or scrollable content areas.

How?

  • Added conditional CSS class is-fill-size to the modal's children container when size="fill" or isFullScreen={true}
  • This class can be targeted by developers to apply min-height: 100% or similar styles to make content fill the available modal height
  • Only applying the class when fill sizing is explicitly requested
  • Added comprehensive tests to verify the class is applied correctly in all scenarios

Testing Instructions

  1. Open the Modal component storybook: https://wordpress.github.io/gutenberg/?path=/story/components-modal--default
  2. Set the modal size to "fill" using the storybook controls
  3. Verify that the modal's children container now has the is-fill-size CSS class
  4. Test with isFullScreen={true} to ensure the class is also applied in fullscreen mode
  5. Verify that regular modal sizes (small, medium, large) do not have the is-fill-size class
  6. Test the new functionality by adding content with height: 100% to see it now properly fills the modal height

Testing Instructions for Keyboard

  1. Navigate to the Modal storybook using Tab key
  2. Use arrow keys to change the size control to "fill"
  3. Verify the modal opens and closes properly with Escape key
  4. Test focus management within the modal using Tab key
  5. Ensure the new CSS class doesn't interfere with existing keyboard navigation

Screenshots

image

@github-actions
Copy link

github-actions bot commented Jul 9, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Mayank-Tripathi32 <[email protected]>
Co-authored-by: talldan <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: mirka <[email protected]>
Co-authored-by: ntsekouras <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@t-hamano t-hamano added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components labels Jul 10, 2025
@Mayank-Tripathi32
Copy link
Contributor Author

unit test failure seems irrelevant, might need to rerun tests.

@Mamaduka
Copy link
Member

Mamaduka commented Nov 9, 2025

@Mayank-Tripathi32, do you mind rebasing this branch on top of the latest trunk?

@Mayank-Tripathi32 Mayank-Tripathi32 force-pushed the try/feat-add-css-in-modal branch from da13a95 to 2de80a6 Compare November 9, 2025 12:16
@Mayank-Tripathi32
Copy link
Contributor Author

@Mamaduka Done. Thanks for the ping!

'components-modal__children-container',
{
'is-fill-size':
size === 'fill' || isFullScreen,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure it should be the default behavior (that might cause things to break for existing users of this component), maybe a prop to set this class would work.

A changelog entry is also needed for the components package.

@talldan talldan requested review from aduth and mirka November 10, 2025 05:32
@mirka
Copy link
Member

mirka commented Nov 18, 2025

We're exploring some comprehensive fixes in #73150. A height: 100% is not quite sufficient even for the Storybook example, as it creates an unnecessary overflow due to the margins on the p element.

@Mamaduka
Copy link
Member

Mamaduka commented Dec 2, 2025

@mirka, can we close this now that #73150 is merged?

@mirka mirka closed this Dec 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package] Components /packages/components [Type] Enhancement A suggestion for improvement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Modal: When 'fill' size is used it's difficult to make content take up the full height of the modal

5 participants