chore(components): upgrade LeafyGreen modal components COMPASS-9642 by kraenhansen · Pull Request #7594 · mongodb-js/compass

kraenhansen · 2025-11-25T10:53:24Z

COMPASS-9642

Description

This PR upgrades the modal related LG packages to their latests version.

Checklist

New tests and/or benchmarks are included
Documentation is changed or added
If this change updates the UI, screenshots/videos are added and a design review is requested
If this change could impact the load on the MongoDB cluster, please describe the expected and worst case impact
I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

Motivation and Context

Bugfix
New feature
Dependency update
Misc

Open Questions

Dependents

Types of changes

Backport Needed
Patch (non-breaking change which fixes an issue)
Minor (non-breaking change which adds functionality)
Major (fix or feature that would cause existing functionality to change)

Copilot

Pull request overview

This PR upgrades LeafyGreen modal-related packages to their latest versions. The upgrade introduces breaking changes in the LeafyGreen Modal API, requiring updates to modal usage patterns throughout the codebase. Key changes include updating modal prop names, refactoring modal state detection in tests, and adjusting focus management behavior.

Updated LeafyGreen modal packages and related dependencies to latest versions
Refactored modal prop usage from deprecated patterns (e.g., contentClassName → className, onButtonClick → buttonProps)
Implemented new modal state detection utilities for E2E tests using dialog element attributes
Updated autofocus handling to use explicit autoFocus props instead of initialFocus

Reviewed changes

Copilot reviewed 102 out of 103 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
packages/compass-components/package.json	Updated LeafyGreen dependencies to latest versions
packages/compass-components/src/components/modals/*	Refactored modal components to use new API patterns
packages/compass-e2e-tests/helpers/commands/*	Added new modal state detection utilities and updated test helpers
packages/compass-e2e-tests/tests/*	Updated E2E tests to use new modal detection methods
configs/testing-library-compass/src/assertions.ts	Added custom chai assertions for dialog open/closed states
configs/mocha-config-compass/register/jsdom-extra-mocks-register.js	Added HTMLDialogElement polyfill for jsdom

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-25T16:45:24Z

packages/compass-welcome/src/components/modal.spec.tsx

    userEvent.click(startButton);
    await waitFor(() => {
-      expect(() => screen.getByTestId('welcome-modal')).to.throw();
+      expect(screen.getByTestId('welcome-modal')).be.closed;


Missing 'to' in the chai assertion chain. Should be expect(screen.getByTestId('welcome-modal')).to.be.closed;

Copilot · 2025-11-25T16:45:24Z

packages/compass-welcome/src/components/modal.spec.tsx

    userEvent.click(closeButton);
    await waitFor(() => {
-      expect(() => screen.getByTestId('welcome-modal')).to.throw();
+      expect(screen.getByTestId('welcome-modal')).be.closed;


Missing 'to' in the chai assertion chain. Should be expect(screen.getByTestId('welcome-modal')).to.be.closed;

Copilot · 2025-11-25T16:45:25Z

packages/compass-welcome/src/components/modal.spec.tsx

    userEvent.click(settingsLink);
    await waitFor(() => {
-      expect(() => screen.getByTestId('welcome-modal')).to.throw();
+      expect(screen.getByTestId('welcome-modal')).be.closed;


Missing 'to' in the chai assertion chain. Should be expect(screen.getByTestId('welcome-modal')).to.be.closed;

kraenhansen · 2025-11-26T08:26:55Z

packages/compass-assistant/src/compass-assistant-provider.spec.tsx

-          expect(screen.getByTestId('assistant-confirm-clear-chat-modal')).to
-            .exist;
+          expect(
+            screen.getByTestId('assistant-confirm-clear-chat-modal').firstChild


Using .firstChild here because the modal root (dialog element) always exists, regardless of it being open or not.

kraenhansen · 2025-11-26T08:29:00Z

configs/testing-library-compass/src/assertions.ts

+declare global {
+  // eslint-disable-next-line @typescript-eslint/no-namespace
+  export namespace Chai {
+    interface Assertion {
+      /** Asserts that a dialog is open */
+      get open(): Assertion;
+      /** Asserts that a dialog is closed */
+      get closed(): Assertion;
+    }
+  }
+}
+
+util.addProperty(
+  Assertion.prototype,
+  'open',
+  function (this: typeof Assertion) {
+    const obj = util.flag(this, 'object');
+    new Assertion(obj).to.be.instanceof(HTMLDialogElement);
+    new Assertion(obj as HTMLDialogElement).has.property(
+      'open',
+      true,
+      'Expected dialog to be open'
+    );
+  }
+);
+
+util.addProperty(
+  Assertion.prototype,
+  'closed',
+  function (this: typeof Assertion) {
+    const obj = util.flag(this, 'object');
+    new Assertion(obj).to.be.instanceof(HTMLDialogElement);
+    new Assertion(obj as HTMLDialogElement).has.property(
+      'open',
+      false,
+      'Expected dialog to be closed'
+    );
+  }
+);


Adding new assertions to determine if a dialog element is open.

I'm putting this here (instead of the @mongodb-js/mocha-config-compass package) since this package is TypeScript and the other is pure JavaScript at the moment.

configs/testing-library-compass/src/assertions.ts

gribnoysup · 2025-11-26T13:30:35Z

packages/compass-assistant/src/compass-assistant-provider.spec.tsx


-        await waitForElementToBeRemoved(() =>
-          screen.getByTestId('assistant-confirm-clear-chat-modal')
-        );


FYI this changes the test because waitForRemoved specifically exists to check first that the element exists before being removed, just waitFor doesn't do it, but I guess it's good enough if we checked before

check first that the element exists before being removed

I agree that does change the test - but also, that behavior seems like a race-condition to me 🤔 It depends on an implementation detail of the userEvent.click(confirmButton); firing (and re-rendering the modal), asynchronously.

This might be an inherent property of the testing library?

It's not related to testing-library, it depends just on how it's rendered on the screen

gribnoysup · 2025-11-26T13:31:35Z

configs/testing-library-compass/src/assertions.ts

+  export namespace Chai {
+    interface Assertion {
+      /** Asserts that a dialog is open */
+      get open(): Assertion;


@Anemy FYI for the eslint plugin you are working on in devtools-shared, new terminator assertion

gribnoysup · 2025-11-26T13:32:47Z

packages/compass-components/src/components/modals/confirmation-modal.spec.tsx

-    renderModal();
-    const button = screen.getByText('Confirm').closest('button');
+    const { container } = renderModal();
+    const button = within(container).getByText('Confirm').closest('button');


I'm curious why this change is required: testing-library supposed to clean-up after itself, so there should be no other elements on the screen with text Confirm in them and using screen over returned values from render is the recommended way to use testing-library (not a strict requirement, but still)

I've reverted this change 👍 It was introduced before mongodb/leafygreen-ui#3189 was merged and released.

gribnoysup · 2025-11-26T13:35:38Z

packages/compass-components/src/components/content-with-fallback.spec.tsx

How did we ended up with context menu (and now modal tests) in this component?

It's also doesn't seem to be used anymore, so might as well just drop this whole thing

It is the behavior of the new modal to include the dialog element, regardless of the modal being open or not.

It's also doesn't seem to be used anymore

What specifically doesn't seem to be used anymore?

Sure, but why this unrelated component that doesn't render either a modal or a context menu is testing that those two are rendered?

What specifically doesn't seem to be used anymore?

Any of the components in this file:

I agree - the value of this test deteriorated beyond being useful. I'd be happy to delete the two components and their tests all together. Would that suffice?

Created #7615

Merged the PR - I believe this thread can be resolved 👍

gribnoysup · 2025-11-26T13:38:57Z

packages/compass-components/src/hooks/use-confirmation.tsx

          children: confirmationProps.buttonText ?? 'Confirm',
          onClick: handleConfirm,
          ...confirmationProps.confirmButtonProps,
+          'data-lgid': 'lg-confirmation_modal-footer-confirm_button',


lgid doesn't look like something we should be touching or overriding, is there no way we can pass our own id or another data attribute to the confirm button so that we don't depend on leafygreen for this? Can we autoFocus it instead like in the other cases in this patch?

I reverted this line and updated the initialFocus prop below:

compass/packages/compass-components/src/hooks/use-confirmation.tsx

Lines 204 to 207 in 406b9a4

(confirmationProps.requiredInputText

? 'auto'

: // TODO: Update this once https://jira.mongodb.org/browse/LG-5735 gets resolved

'[data-testid=lg-confirmation_modal-footer-cancel_button]')

This aligns with an upcoming change in LG (https://jira.mongodb.org/browse/LG-5735)

gribnoysup · 2025-11-26T13:50:49Z

packages/compass-e2e-tests/helpers/commands/wait-for-open-modal.ts

+        for (const modal of modals) {
+          // Ensure any modals are interactable if open
+          await modal.waitForClickable({
+            timeout: 500,


Unexpected to see defaults overriden in shared helpers, we configure them for the whole test suite via env usually because different environments might require slightly different configuration (we're also pretty generous with those usually especially for CI), so setting it to a value different from default by default doesn't seem right to me: it might be 500ms on your machine, but 5s next time windows host in GHA has issues with performance

I agree and updated this to pass these options instead of overriding the default:

compass/packages/compass-e2e-tests/helpers/commands/wait-for-open-modal.ts

Lines 10 to 35 in 406b9a4

await browser.waitUntil(

async () => {

const modals = await browser.getOpenModals(selector);

const count = await modals.length;

if (reverse) {

return count === 0;

} else if (count === 0) {

return false;

} else {

for (const modal of modals) {

// Ensure any modals are interactable if open

await modal.waitForClickable({

timeoutMsg: 'Timeout waiting for open modal to become clickable',

...options,

});

}

return true;

}

},

{

timeoutMsg: `Timeout waiting for modal '${inspect(selector)}' to ${

reverse ? 'close' : 'open'

}`,

...options,

}

);

gribnoysup · 2025-11-26T13:59:35Z

packages/compass-e2e-tests/helpers/commands/wait-for-open-modal.ts

+): Promise<void> {
+  await browser.waitUntil(
+    async () => {
+      const modals = await browser.getOpenModals(selector);


I would expect that if I'm calling waitForOpenModal, I'm only looking for one modals, weird to see that this code is dealing with multiple ones instead of throwing if it got more than one back. Is this some quirk of how leafygreen renders those that forces us to do this? Is it just always adding a new modal on the screen or something instead of re-rendering?

Is this some quirk of how leafygreen renders those that forces us to do this?

The selector is often matching a data-testid which is put on the dialog element, which is mounted into the DOM even when the modal is closed. If two components rendering a modal internally is mounted at the same time, multiple elements will match the selector. Waiting for the modal to open then becomes a matter waiting for one of those modals to open.

One alternative I considered was appending a [open] attribute to the selector. I ultimately scrapped that because it felt too brittle. For one, it would not work for selector lists (selectors separated with ,) 🤔 It would be great if WebDriver IO had a way to impose "additional constraints". This is why I'm currently filtering the modals based on their open attribute in getOpenModals:

compass/packages/compass-e2e-tests/helpers/commands/get-open-modals.ts

Lines 21 to 22 in 3e406d7

const open = await element.getAttribute('open');

return open === 'true';

gribnoysup · 2025-11-26T14:07:23Z

packages/compass-components/src/hooks/use-confirmation.tsx

+        initialFocus={
+          confirmationProps.initialFocus ??
+          '[data-lgid=lg-confirmation_modal-footer-confirm_button]'
+        }


This changes the behavior when confirmation modal is used with required input in an unexpected way and puts focus on a fake-disabled button instead of input as it happens right now. Same for showPrompt where the focus should land on the input we're showing, not on confirm button right away

showPrompt still doesn't focus on the input

kraenhansen · 2025-11-28T13:36:31Z

I added a more robust "locator strategy" to find open dialogs:

compass/packages/compass-e2e-tests/helpers/dialog-open-locator-strategy.ts

Lines 1 to 17 in 406b9a4

    
           export function dialogOpenLocator(selector: string) { 
        
             // eslint-disable-next-line no-restricted-globals -- This runs in the browser 
        
             const result: HTMLDialogElement[] = []; 
        
             // eslint-disable-next-line no-restricted-globals -- This runs in the browser 
        
             const elements = document.querySelectorAll(selector); 
        
             for (let i = 0; i < elements.length; i++) { 
        
               const element = elements[i]; 
        
               if ( 
        
                 // eslint-disable-next-line no-restricted-globals -- This runs in the browser 
        
                 element instanceof HTMLDialogElement && 
        
                 element.open 
        
               ) { 
        
                 result.push(element); 
        
               } 
        
             } 
        
             return result; 
        
           }

The benefit of this is that all the code to select and filter runs in the browser, instead of passing back and forth between the webdriver client and the browser using async operations. This makes the check much more reliable when elements are entering and leaving the document.

… more important with the new dialog

…ny case

…to move the code later

kraenhansen · 2025-12-18T21:14:28Z

Bulk update modal still needs adjustment because there is a noticeable visual issue with the modal sticking to the screen border and we should still remove data-testid usage from application logic, but otherwise this looks good now

I fixed the "bulk update modal issue" with c2e5d46 and updated the confirmation modal to "remove data-testid usage from application logic" 👍

kraenhansen self-assigned this Nov 25, 2025

kraenhansen added dependencies Pull requests that update a dependency file no release notes Fix or feature not for release notes labels Nov 25, 2025

kraenhansen force-pushed the kh/leafygreen-non-modal-updates branch from 7b62353 to 694ac4e Compare November 25, 2025 14:15

Base automatically changed from kh/leafygreen-non-modal-updates to main November 25, 2025 16:12

kraenhansen marked this pull request as ready for review November 25, 2025 16:44

kraenhansen requested a review from a team as a code owner November 25, 2025 16:44

kraenhansen requested review from Copilot and nbbeeken November 25, 2025 16:44

Copilot AI reviewed Nov 25, 2025

View reviewed changes

kraenhansen force-pushed the kh/leafygreen-modal-updates branch 2 times, most recently from bc02a6d to 4467057 Compare November 25, 2025 17:06

kraenhansen requested a review from gribnoysup November 25, 2025 17:07

kraenhansen force-pushed the kh/leafygreen-modal-updates branch from bac36cb to 8c0d915 Compare November 26, 2025 08:25

kraenhansen commented Nov 26, 2025

View reviewed changes

gribnoysup reviewed Nov 26, 2025

View reviewed changes

configs/testing-library-compass/src/assertions.ts Show resolved Hide resolved

gribnoysup reviewed Nov 26, 2025

View reviewed changes

kraenhansen mentioned this pull request Nov 26, 2025

chore(components): upgrade LeafyGreen chat components COMPASS-9642 #7610

Merged

13 tasks

kraenhansen force-pushed the kh/leafygreen-modal-updates branch 5 times, most recently from 3595882 to 406b9a4 Compare November 28, 2025 12:56

kraenhansen mentioned this pull request Nov 28, 2025

chore(components): delete unused ContentWithFallback and FadeInPlaceholder components #7615

Merged

13 tasks

kraenhansen requested a review from gribnoysup November 28, 2025 14:23

kraenhansen force-pushed the kh/leafygreen-modal-updates branch 3 times, most recently from e822695 to a8bf853 Compare December 1, 2025 14:02

Kræn Hansen added 25 commits December 18, 2025 18:39

Update databases-collections to allow immediate removal of the heading

e473a04

Update e2e tests to use waitForDisplayed over waitForExist as this is…

01c66a3

… more important with the new dialog

Update e2e tests removing [role=dialog] from confirm dialog selectors

b343940

Update e2e tests to wait for bulk delete modal to disappear

3cdb6c7

Update e2e tests to wait for connection form to be non-clickable in a…

ad72a99

…ny case

Update e2e tests to fix modal regressions

5e5ce3b

Disable pointer events on a closed modal

2778a71

Disable auto-focus on confirmation modal children by default

d1fe84b

Add new modal specific commands

6ab317e

Update E2E tests to use new modal commands instead of waitForDisplayed

70e7015

Default initialFocus to "null" to avoid breaking existing modals

f839793

Wait for confirm button to show instead of simply expecting it

cf032a1

Update FocusMode tests to handle LG Modal v20

bb6ff36

Update SettingsModal tests to handle LG Modal v20

3799746

Clarify disabling the @typescript-eslint/no-namespace and add a TODO …

7ca774e

…to move the code later

Partial revert of ddd6d70

de78fec

Fix auto-focus in bulk update and delete modals

920efc6

Correcting sizing styles

2f73260

Correcting sizing styles more

c2e5d46

Use named imports

9646d87

Upgrade confirm-modal package

d43fc16

Fix useConfirmation's confirmButtonProps types

dbe7574

Update use of ConfirmationModal

9767e4f

Set initial focus on cancel (fallback to confirm) button

564fc1e

Fix package lock

6fa377d

kraenhansen force-pushed the kh/leafygreen-modal-updates branch from 536b433 to 6fa377d Compare December 18, 2025 17:39

Anemy approved these changes Dec 18, 2025

View reviewed changes

kraenhansen merged commit 344aa9c into main Dec 19, 2025
62 checks passed

kraenhansen deleted the kh/leafygreen-modal-updates branch December 19, 2025 08:23

	(confirmationProps.requiredInputText
	? 'auto'
	: // TODO: Update this once https://jira.mongodb.org/browse/LG-5735 gets resolved
	'[data-testid=lg-confirmation_modal-footer-cancel_button]')

	await browser.waitUntil(
	async () => {
	const modals = await browser.getOpenModals(selector);
	const count = await modals.length;
	if (reverse) {
	return count === 0;
	} else if (count === 0) {
	return false;
	} else {
	for (const modal of modals) {
	// Ensure any modals are interactable if open
	await modal.waitForClickable({
	timeoutMsg: 'Timeout waiting for open modal to become clickable',
	...options,
	});
	}
	return true;
	}
	},
	{
	timeoutMsg: `Timeout waiting for modal '${inspect(selector)}' to ${
	reverse ? 'close' : 'open'
	}`,
	...options,
	}
	);

	const open = await element.getAttribute('open');
	return open === 'true';

Conversation

kraenhansen commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Motivation and Context

Open Questions

Dependents

Types of changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

kraenhansen Nov 26, 2025 • 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!

kraenhansen Nov 26, 2025 • 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!

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!

kraenhansen Nov 26, 2025 • 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!

gribnoysup Nov 26, 2025 • 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!

kraenhansen commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

kraenhansen commented Nov 25, 2025 •

edited

Loading

kraenhansen Nov 26, 2025 •

edited

Loading

kraenhansen Nov 26, 2025 •

edited

Loading

kraenhansen Nov 26, 2025 •

edited

Loading

gribnoysup Nov 26, 2025 •

edited

Loading

kraenhansen commented Nov 28, 2025 •

edited

Loading