Components: Modal Height Controls

[chad1008]

We recently implemented some preset sizes for Modal via the size prop: small, medium, large, and fill. (fill is analogous to and will eventually replace the isFullScreen prop)

See #54471 for more, but the goal of this change was to make it easier for consumers to specify a width for their modals without having to resort to excessive style overrides.

An example of where this can be helpful is the font library modal in the site editor, where a temporary workaround is currently in place.

This is an interesting case, because while our new prop was primarily focused on widths (to avoid things like paragraph tags wouldn't push the modal to full viewport width), the font library also has height needs. Right now it's using isFullScreen, which creates a modal that is... you guessed it... full screen, with an a small buffer on each side. Font library is then explicitly limiting the width to 65vw. That means isFullScreen is really only in place to make the modal taller than it would be based on content alone.

The new size = "large" option would be a good fit here, giving a workable width on medium and large devices but allowing a full screen mobile experience. Except for the height issue. The problem is that the different tabs contained within this particular Modal are all different heights. That means switching tabs creates an unsightly jumping effect when size: large is applied:

Screen.Recording.2023-10-04.at.16.10.58.mov

Having a consistent height that can be specified by the consumer would be helpful here. It can be accomplished with CSS, but would it be preferable for Modal provide a built-in solution?

Possible ideas:

• a new prop for height, maybe something like isFullHeight that would have the sam height effect of isFullScreen, but not impact width at all.
• apply a height value to our existing presets. Maybe just have large fill the screen vertically, maybe set specific min-heights for each?

Curious what others think from a component/design perspective. cc @ciampo @brookewp @andrewhayward @WordPress/gutenberg-design

[ciampo]

I guess we could at most set some proportionate min-height based on the width, but I'd be quite conservative with it

[jasmussen]

How about trying an aspect-ratio?

[ciampo]

aspect-ratio seems to work — it sets a min-height, but it grows as the container grows. I put together a quick demo, you can add / remove text to see the how it would behave

It wouldn't work if we wanted to impose a fixed height (although I'm not sure it would be a good approach)

[jasmussen]

aspect-ratio should respect a max-height, though, and it might be good to have one. It can be a very tall max-height, such as calc(100svh - 4rem), something in that vein, but we wouldn't want it to grow taller than the screen.

[jameskoster]

Aspect ratio with a max-height could work as an option, but I think the default behavior should remain otherwise shorter modals become unnecessarily tall:

Still, this particular issue seems to be fairly contextual, based on the inclusion of sub-navigation where each page affects the overall height. I wonder if a more comprehensive solution would be to build this feature into the Modal. It would avoid potential misuse of an aspect ratio option and promote consistent navigation patterns within modals.

[andrewhayward]

Respecting a desired height with aspect-ratio is definitely doable with a liberal application of overflow: auto;, but I'm not totally sure how that would work either!

... {
  ...

  width: 200px/400px/600px/800px;
  aspect-ratio: 2/1;

  min-width: 33dvw;
  max-width: calc(100dvw - 2rem);
  min-height: 300px;
  max-height: calc(100dvh - 2rem);
}

[ciampo]

I wonder if a more comprehensive solution would be to build this feature into the Modal.

@jameskoster what do you refer to when you mention "this solution" ?

I think that this conversation is already about potentially adding a preferred aspect ratio to the existing size presets.

I'd also caution us against adding too many specific styles to a generic component like Modal.

[jameskoster]

@ciampo never mind, my point was that you'd only need the modal height to be fixed when there's a sub-navigation (like the fonts example in the OP, or the Preferences modal in the editor). But thinking about it some more there are other legitimate uses cases, like searching within a modal.

If the aspect-ratio idea doesn't work out, another option could be to try the command palette approach. There the modal height still 'hugs' its contents, but since it's a fixed distance from the top of the viewport (rather than centrally aligned) it works okay.

[richtabor]

What if instead the top of the modal was positioned consistently, regardless of content height? Instead of an absolute center.

[chad1008]

aspect-ratio feels really promising, but it does look like it might require moving Modal away from using display: flex.

Based on some testing I've done just now, when flex is involved, the aspect-ratio is more strictly enforced, and the height of the element no longer expands with the content.

When combined with the Modal content's overflow:auto, aspect-ratio effectively becomes a fixed height that automatically makes the content scrollable. Here's a slight modification of @ciampo's previous example to demonstrate.

I can certainly look into removing flexbox from Modal to make room for aspect-ratio, unless that's a non-starter for anyone.

If that doesn't sound like a good path, maybe we apply a fixed hight to just one of the presets? Maybe large should always be full height (ie the calc(100% - 80px) we apply for size: fill/isFullScreen? That or a separate prop to make any modal full screen height?

[ciampo]

I think I'm losing track of exactly what issue are trying to solve here.

Looking at the initial example at the top of this issue, to me it looks okay that the consumer of Modal needs to set a specific height on the component for this specific use case.

I get the convenience of offering some preset sizes, and why we may also want to add height to those presets. But we can not expect to cover all use cases of Modal — which is why we allow folks to set a classname on it, and customize it.

[chad1008]

I think I'm losing track of exactly what issue are trying to solve here.

My only concern was that in cases where the height of the Modal contents changes, like with internal tabs/navigation, the size of the modal can bounce around in an unsightly way.

If we think that's something we'd prefer to let consumers manage directly via CSS, that works for me, but I wanted to raise the question once I saw it.

[richtabor]

My only concern was that in cases where the height of the Modal contents changes, like with internal tabs/navigation, the size of the modal can bounce around in an unsightly way.

Why I was thinking of a solution that maintains the origin positioning, regardless of height change.

[ciampo]

Got it, thank you both!

I guess one solution wouldn't prevent the other?

• We could work on a prop which toggles the origin positioning (as suggested by @richtabor )
• Separately, we could investigate adding some form of height constraint (as discussed by @chad1008 )

One thing I'm going to say is that we're about to start working on a new version of the Modal component, powered by a third party library (likely ariakit) instead of our current custom first-party implementation. I think it may be smart to punt any of the changes discussed in this issue so that we can apply them directly to the new version of the component?

[chad1008]

Cool. I can plan on tackling it that way, and in the meantime just leave the font-library hight fixed as it is today 👍

Thanks everyone!!