From f29012ab755e3062b7fd86348e90d73d76145f96 Mon Sep 17 00:00:00 2001 From: "Ronald A. Richardson" Date: Thu, 20 Aug 2026 11:28:45 +0800 Subject: [PATCH] =?UTF-8?q?Batch=20B=20part=20one:=20modal.js=20=E2=80=94?= =?UTF-8?q?=20FastBoot=20residue,=20and=20two=20arguments=20the=20modal=20?= =?UTF-8?q?never=20resolved?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit modal.js was the largest single remaining file (13 statements / 14 branches). Its gap turned out to be three unrelated things, only one of which was a real test gap. Tests (4 new). `` declares `keyboard` and `backdropClose` and passes them to , which reads them only from inside `handleKeyDown` and `handleClick`. Glimmer evaluates arguments lazily, so until one of those handlers fires through a real , the modal's own `@arg` defaults are never resolved — which is why they read as never-initialised. The existing dialog tests render directly with the arguments passed explicitly, so they never exercised the modal's side of it. Added: escape closing the modal by default and being ignored under @keyboard={{false}}, and the same pair for clicking the backdrop area. This is the same lazy-argument shape as layout/resource/panel's `authSchema` in the previous batch. Worth recognising on sight: an argument that is only read inside a handler is not evaluated until that handler runs. Exclusions (6): every `isFastBoot(this)` guard, plus the SimpleDOM branch of `addBodyClass` behind one of them. This suite runs in a browser, where isFastBoot() is always false, so neither arm can be reached. Left uncovered and NOT excluded — six teardown and re-entrancy guards: `_isOpen`, three `isDestroyed` checks sitting after awaits, and two `!modalElement` checks. These are the same shape as chart's teardown guard, which was genuinely reachable and got a test last batch. Reaching these needs the component destroyed mid-transition, and `render()` awaits settled, so the transition has already finished before a test could tear it down. They are reachable in principle, so excluding them would be wrong; recorded here instead. Full suite 5018 pass / 0 fail / 0 skip. Coverage 94.13% statements, 90.06% branches, 97.45% functions, 94.55% lines. Branches are past 90% for the first time. Batch B remaining after this file: 79 files, ~308 statements, ~357 branches. --- addon/components/modal.js | 6 +++ tests/integration/components/modal-test.js | 47 +++++++++++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/addon/components/modal.js b/addon/components/modal.js index b482633e..3808f512 100644 --- a/addon/components/modal.js +++ b/addon/components/modal.js @@ -450,6 +450,7 @@ export default class Modal extends Component { return; } + /* istanbul ignore next -- FastBoot-only: this suite runs in a browser, where isFastBoot() is always false. */ if (!isFastBoot(this)) { this.checkScrollbar(); this.setScrollbar(); @@ -462,6 +463,7 @@ export default class Modal extends Component { return; } + /* istanbul ignore next -- FastBoot-only: this suite runs in a browser, where isFastBoot() is always false. */ if (!isFastBoot(this)) { modalElement.scrollTop = 0; this.adjustDialog(); @@ -513,6 +515,7 @@ export default class Modal extends Component { this.removeBodyClass(); + /* istanbul ignore next -- FastBoot-only: this suite runs in a browser, where isFastBoot() is always false. */ if (!isFastBoot(this)) { this.resetAdjustments(); this.resetScrollbar(); @@ -633,6 +636,7 @@ export default class Modal extends Component { // Only add the body class if this is the first modal if (this.modalsManager.modals.length === 1) { // special handling for FastBoot, where real `document` is not available + /* istanbul ignore next -- FastBoot-only: this suite runs in a browser, where isFastBoot() is always false. */ if (isFastBoot(this)) { // a SimpleDOM instance with just a subset of the DOM API! let document = this.document; @@ -653,6 +657,7 @@ export default class Modal extends Component { removeBodyClass() { // Only remove the body class if there are no more modals if (this.modalsManager.modals.length === 0) { + /* istanbul ignore next -- FastBoot-only: this suite runs in a browser, where isFastBoot() is always false. */ if (isFastBoot(this)) { // no need for FastBoot support here return; @@ -683,6 +688,7 @@ export default class Modal extends Component { this.removeBodyClass(); + /* istanbul ignore next -- FastBoot-only: this suite runs in a browser, where isFastBoot() is always false. */ if (!isFastBoot(this)) { this.resetScrollbar(); } diff --git a/tests/integration/components/modal-test.js b/tests/integration/components/modal-test.js index ccb20d50..e06bfc31 100644 --- a/tests/integration/components/modal-test.js +++ b/tests/integration/components/modal-test.js @@ -1,6 +1,6 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'dummy/tests/helpers'; -import { render, click, settled, find, findAll } from '@ember/test-helpers'; +import { render, click, settled, find, findAll, triggerKeyEvent } from '@ember/test-helpers'; import { hbs } from 'ember-cli-htmlbars'; const DIALOG = '.flb--modal'; @@ -244,4 +244,49 @@ module('Integration | Component | modal', function (hooks) { assert.dom('.flb--modal-backdrop').hasClass('fade'); }); + // declares `keyboard` and `backdropClose` and hands them to , which + // only reads them from inside its keydown and click handlers. Glimmer evaluates arguments + // lazily, so until one of those handlers actually runs through a real , the modal's own + // defaults are never resolved. + module('the dialog arguments the modal supplies', function () { + test('escape closes the modal by default', async function (assert) { + const closes = []; + this.set('onClose', () => closes.push('closed')); + + await render(hbs`body`); + await triggerKeyEvent(DIALOG, 'keydown', 'Escape'); + + assert.deepEqual(closes, ['closed'], 'the default keyboard handling is on'); + }); + + test('escape is ignored when keyboard handling is switched off', async function (assert) { + const closes = []; + this.set('onClose', () => closes.push('closed')); + + await render(hbs`body`); + await triggerKeyEvent(DIALOG, 'keydown', 'Escape'); + + assert.deepEqual(closes, [], 'the modal stays open'); + }); + + test('clicking the backdrop area closes the modal by default', async function (assert) { + const closes = []; + this.set('onClose', () => closes.push('closed')); + + await render(hbs`body`); + await click(DIALOG); + + assert.deepEqual(closes, ['closed'], 'the default backdrop-close is on'); + }); + + test('clicking it is ignored when backdropClose is switched off', async function (assert) { + const closes = []; + this.set('onClose', () => closes.push('closed')); + + await render(hbs`body`); + await click(DIALOG); + + assert.deepEqual(closes, [], 'the modal stays open'); + }); + }); });