feat(components): Modal set max height and allow scrolling #2865

edison-cy-yang · 2026-01-07T00:15:05Z

Motivations

composable Modal does not have a max height, causing Modal with a lot of content to render off screen and use the browser scrollbar to navigate.

This change will set the max height of Modal to be the viewport height, minus some padding. The scrollbar will also now exist on the Modal itself.

We have plans to update Modal so only the middle content is scrollable, while Modal.Header and Modal.Actions stay fixed. That requires more thoughts and planning in order to work with the current structure of Modal, so it will come as a future enhancement.

Changes

Added

Visual test for Modal using provider with a lot of content

Changed

Modal max height is set to viewport height, minus space-base
overflow-y is set on the floating element

Deprecated

Removed

Fixed

Security

Testing

Before

After

Changes can be
tested via Pre-release

In Atlantis we use Github's built in pull request reviews.

github-actions · 2026-01-07T20:36:45Z

Published Pre-release for 64f65d9 with versions:

  - @jobber/components@6.106.4-JOB-147120-64f65d9.18+64f65d9f0

To install the new version(s) for Web run:

npm install @jobber/components@6.106.4-JOB-147120-64f65d9.18+64f65d9f0

cloudflare-workers-and-pages · 2026-01-09T22:30:16Z

Deploying atlantis with Cloudflare Pages

Latest commit:	`64f65d9`
Status:	✅ Deploy successful!
Preview URL:	https://9ce62f08.atlantis.pages.dev
Branch Preview URL:	https://job-147120-set-modal-max-hei.atlantis.pages.dev

View logs

This reverts commit ee65a06.

…ference element

edison-cy-yang · 2026-01-12T21:46:05Z

So I ran into some issue with using the size middleware to set max height. Floating UI's size middleware depends on computePosition to run, which in turn requires a reference element. When Modal.Activator is not provided, the reference element is null, causing the middleware to never execute and max-height is not applied.

It also does not make sense for us to force a reference element since the floating modal does not anchor to anything when opened, it just takes up the center of the viewport.

Therefore, I went back to plain CSS to set max height. The only downside of this approach is it will not react to parent clipping container. However, Modal is not supposed to be used in that context anyways.

ZakaryH · 2026-01-14T17:46:58Z

packages/components/src/Modal/Modal.rebuilt.module.css

 }

 .modal {
+  display: flex;


is this necessary? I ask because since this style is exported via useModalStyles if someone is using this style, it could be a notable change for their layout.

we do have at least one instance of someone using the modal exported style, so I have no idea what the consequences of these changes will be on their layout.

I really would prefer not to expose our styles like this.

You're right, initially flex was added so modalBody does not expand to fit its content. Alternatively, I took it out and set .modalBody's max-height to inherit from .modal.

ZakaryH · 2026-01-14T17:48:21Z

packages/components/src/Modal/Modal.rebuilt.module.css

+
+.modalBody {
+  min-height: 0;
+  padding: var(--space-small);


I wouldn't mind if we just removed this. if I'm understanding it right, this is the padding that used to be on .modal

however, that padding doesn't make sense. it's extra padding that we don't need.

I'll double check I'm understanding it right though.

actually it's fine. even though I don't like it, it has nothing to do with these changes. we can keep it equivalent for the time being and address it later.

Ideally this should be the overall padding of the modal and eventually we don't set margin on header, body, and actions separately.

true. we need to either commit to the individual pieces providing the padding, or do that.

I have never understood our current state that is a mixture of everything 😵

padding on Header, and Actions

no padding on the main content

extra padding on the entire thing

ZakaryH · 2026-01-14T17:49:18Z

packages/components/src/Modal/Modal.rebuilt.module.css

+  overflow-y: auto;
+}
+
+.modalBody:focus-visible {


I'm not sure if this is equivalent if it works how I think it might. the focus visible needs to be on the entire modal, not just the body.

ok yeah we can't move this down, it needs to be on the actual overall modal container

here's prod

vs

the outline is gone, because focus is programmatically moved to the actual modal container, and that's what has to respond to the focus-visible selector

once we adjust this, we should verify the extra div doesn't interfere with tab order too.

Moved it back to modal in 8fb48cc and confirmed the div does not interfere with tab order

on Firefox, I am seeing one extra tab being required. I think it's from the new div? I need to validate that idea but on prod I'm not seeing this extra tab.

In the story there is a Menu in the header. But yeah once screen reader is on it correctly tabs to the interactive element in the header.

oh right. that Menu in the header is an anti pattern. we should not be doing that in the first place, nor encouraging that UX. I need to make a ticket for that. it introduces a lot of other issues.

wait, while it does have issues - I'm able to tab to it on Safari on prod. do you have tab navigation turned on in Safari? by default it's off.

I didn't have it on. Now it's on and all good

tabIndex added in e876207

ZakaryH · 2026-01-14T17:51:06Z

packages/components/src/Modal/Modal.rebuilt.tsx

                    }}
                  >
-                    {children}
+                    <div className={modalBody}>{children}</div>


bit of a nitpick for the naming. is this really the modal body?

the children in here will be everything right? ie. the header, the content/body, and the actions.

Yeah the children will be the header, content, and actions. The ideal name should really be modalContent, but I'm not sure if that's confusing to be so many layers down in a ModalContent component.

that's fair. I suppose as long as we don't expose this externally, it doesn't make much of a difference. we can always change it later if we want.

ZakaryH · 2026-01-14T17:52:49Z

packages/components/src/Modal/useModalStyles.ts

 export function useModalStyles(size?: keyof typeof sizes) {
  return {
    modal: classnames(styles.modal, size && sizes[size]),
+    modalBody: styles.modalBody,


I'm hesitant to add to this pattern of exposing our internal styles for re-use. as mentioned in the .modal style change, by exporting these we make our internal changes unsafe to anyone using this style.

I don't really like doing this, and I think there are better ways to solve the problem it seeks to solve that don't have as many risks and downsides.

Honestly I was caught off guard by this useModalStyles initially and didn't realize its purpose was to expose reusable modal styles.

I removed modalBody and imported styles directly in the component 906a958

nice, yeah I think we should revisit this approach because it creates some problems for us and it's not communicated to us nor consumers that this isn't entirely safe to change.

ZakaryH · 2026-01-14T17:54:53Z

might be worth calling out the difference in our Modal docs, that original Modal doesn't handle content that exceeds the screen size whereas the other one does.

…sions

edison-cy-yang · 2026-01-15T23:14:33Z

might be worth calling out the difference in our Modal docs, that original Modal doesn't handle content that exceeds the screen size whereas the other one does.

Definitely, I added a small paragraph in the docs. I think ideally we should have more documentation that explains the two Modal versions.

ZakaryH · 2026-01-20T18:47:40Z

packages/components/src/Modal/Modal.rebuilt.module.css

  z-index: var(--elevation-modal);
  width: 100%;
  max-width: var(--modal--width);
+  max-height: calc(100vh - 2 * var(--space-base));


one thing I wanna double check is how this behaves on mobile, specifically Safari. I know on there the view height has some quirks, like not taking into account the keyboard presence.

also, thinking about mobile web - I realize this component has other significant issues on mobile. the max-width could be much bigger than the screen size available. that's 100% existing though.

another thing for mobile web is that if we don't have a dismiss button, it can become very difficult to close the modal. the only way to close it will be by tapping the background, but that will only be those 2 spaces above and below.

I'll focus on the vh for this PR and ticket those other issues.

unfortunately this can create an impossible situation in safari mobile

I have no way to dismiss the Modal. I suppose we technically already have this problem, but if we can apply a fix that also covers this that would be much better.

if I use dvh instead of vh it seems to work. I just need to clarify what all the implications are of that, I vaguely recall there being some nuance to that unit.

Changed to dvh and confirmed that there are enough spacing to dismiss modal in mobile web 64f65d9

ZakaryH

two relatively small things:

tabIndex={-1} on the div for Firefox
dvh instead of vh so that (iOS) mobile web can avoid some very bad situations

will want to regenerate the snapshots after just to be sure they're up to date, though it really shouldn't change anything for desktop

edison-cy-yang · 2026-01-20T22:17:16Z

two relatively small things:

tabIndex={-1} on the div for Firefox

dvh instead of vh so that (iOS) mobile web can avoid some very bad situations

will want to regenerate the snapshots after just to be sure they're up to date, though it really shouldn't change anything for desktop

visual regression snapshots have not changed

ZakaryH

thanks for addressing those changes

LGTM

set max height in floating-ui middleware

dc74fc6

test edgePadding change for pre-release

f23d7b5

edison-cy-yang added 5 commits January 9, 2026 15:19

add whileElementsMounted autoUpdate

bccb8f5

remove max height when scroll height is less than availableHeight

b34a302

move floating ref to an outer div that wraps the motion div

ee65a06

Revert "move floating ref to an outer div that wraps the motion div"

77b12c2

This reverts commit ee65a06.

use css to set max height since middleware will not work without a re…

275d629

…ference element

edison-cy-yang added 4 commits January 12, 2026 14:17

create wrapper div to fix scrollbar bleeding over border radius

131e11e

visual test

1a47181

update modal visual snapshots

aeefadf

update site snapshots

3ff60ef

edison-cy-yang marked this pull request as ready for review January 13, 2026 17:59

edison-cy-yang requested a review from a team as a code owner January 13, 2026 17:59

jobbiebot bot added review requested Team UX Foundations labels Jan 13, 2026

ZakaryH reviewed Jan 14, 2026

View reviewed changes

edison-cy-yang added 4 commits January 15, 2026 11:56

remove flex on .modal and set max height inherit on .modalBody

61f1209

moved :focus-visible back onto .modal

8fb48cc

dont expose modalBody useModalStyles

906a958

add documentation for scrolling behavior differences in two modal ver…

1f41e80

…sions

edison-cy-yang requested a review from ZakaryH January 15, 2026 23:31

ZakaryH reviewed Jan 20, 2026

View reviewed changes

add tabIndex to the new div

e876207

ZakaryH requested changes Jan 20, 2026

View reviewed changes

jobbiebot bot added changes requested and removed review requested labels Jan 20, 2026

use dvh to handle mobile browser not having space to close modal

64f65d9

edison-cy-yang requested a review from ZakaryH January 20, 2026 22:17

jobbiebot bot added review requested and removed changes requested labels Jan 20, 2026

ZakaryH approved these changes Jan 20, 2026

View reviewed changes

jobbiebot bot added approved and removed review requested labels Jan 20, 2026

edison-cy-yang merged commit 1b0b048 into master Jan 20, 2026
17 checks passed

edison-cy-yang deleted the JOB-147120-set-modal-max-height-and-allow-scrolling branch January 20, 2026 23:57

feat(components): Modal set max height and allow scrolling #2865

feat(components): Modal set max height and allow scrolling #2865

Uh oh!

Conversation

edison-cy-yang commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivations

Changes

Added

Changed

Deprecated

Removed

Fixed

Security

Testing

Uh oh!

github-actions bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cloudflare-workers-and-pages bot commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying atlantis with Cloudflare Pages

Uh oh!

edison-cy-yang commented Jan 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZakaryH Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZakaryH commented Jan 14, 2026

Uh oh!

edison-cy-yang commented Jan 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

edison-cy-yang commented Jan 7, 2026 •

edited

Loading

github-actions bot commented Jan 7, 2026 •

edited

Loading

cloudflare-workers-and-pages bot commented Jan 9, 2026 •

edited

Loading

ZakaryH Jan 20, 2026 •

edited

Loading