[SCF-2415] [Part 10] Fix modal footer submission and BsAlert @visible regressions

.github/workflows/ci.yml

@@ -3,7 +3,7 @@ name: CI
 on:
   push:
     branches:
-      - '*'
+      - scf_master
     tags:
       - '*'
   pull_request:

addon/components/bs-alert.hbs

@@ -1,8 +1,7 @@
 <div
   class="{{unless this.hidden "alert"}} {{if this.fade "fade"}} {{if this.dismissible "alert-dismissible"}} {{bs-type-class "alert" @type}} {{if this.showAlert (if (macroCondition (macroGetOwnConfig "isNotBS3")) "show" "in")}}"
   ...attributes
-  {{did-update this.showOrHide this._visible}}
-  {{did-update this.updateVisibility @visible}}
+  {{did-update this.showOrHide this.visible}}
 >
   {{#unless this.hidden}}
     {{#if this.dismissible}}

addon/components/bs-alert.js

@@ -1,7 +1,7 @@
 import { action } from '@ember/object';
 import Component from '@glimmer/component';
 import { tracked } from '@glimmer/tracking';
-import { later } from '@ember/runloop';
+import { later, schedule } from '@ember/runloop';
 import usesTransition from 'ember-bootstrap/utils/decorators/uses-transition';
 import deprecateSubclassing from 'ember-bootstrap/utils/deprecate-subclassing';
 import arg from 'ember-bootstrap/utils/decorators/arg';
@@ -86,15 +86,33 @@ export default class Alert extends Component {
    * @public
    */
   get visible() {
-    return this._visible ?? true;
+    let visible = this.args.visible;
+    if (visible !== this._visibleArg) {
+      // untracked bookkeeping, so the alert can be shown again after being dismissed once `@visible` changes
+      // eslint-disable-next-line ember/no-side-effects
+      this._visibleArg = visible;
+      this._visibleArgChanges++;
+    }
+    if (this._dismissedAt === this._visibleArgChanges) {
+      return false;
+    }
+    return visible !== undefined ? visible : true;
   }
 
   /**
-   * @property _visible
+   * `@visible` as last seen by `visible`, and how often it has changed. These are untracked, as `visible` updates them
+   * while rendering.
+   * @private
+   */
+  _visibleArg = this.args.visible;
+  _visibleArgChanges = 0;
+
+  /**
+   * The value of `_visibleArgChanges` when the alert was dismissed, so the alert stays hidden until `@visible` changes.
    * @private
    */
   @tracked
-  _visible = this.args.visible;
+  _dismissedAt = -1;
 
   /**
    * Set to false to disable the fade out animation when hiding the alert.
@@ -163,7 +181,7 @@ export default class Alert extends Component {
   @action
   dismiss() {
     if (this.args.onDismiss?.() !== false) {
-      this._visible = false;
+      this._dismissedAt = this._visibleArgChanges;
     }
   }
 
@@ -202,7 +220,12 @@ export default class Alert extends Component {
       );
     } else {
       this.hidden = true;
-      this.args.onDismissed?.();
+      // hide() runs while rendering, and onDismissed may set @visible, which that render has already read
+      schedule('afterRender', this, function () {
+        if (!this.isDestroyed) {
+          this.args.onDismissed?.();
+        }
+      });
     }
   }
 
@@ -214,9 +237,4 @@ export default class Alert extends Component {
       this.hide();
     }
   }
-
-  @action
-  updateVisibility() {
-    this._visible = this.args.visible;
-  }
 }

addon/components/bs-modal/footer.hbs

@@ -5,7 +5,7 @@
     {{else}}
       {{#if @submitTitle}}
         <Button @onClick={{@onClose}}>{{bs-default @closeTitle "Ok"}}</Button>
-        <Button @type={{bs-default @submitButtonType "primary"}} @buttonType="submit" disabled={{bs-default @submitDisabled false}}>{{@submitTitle}}</Button>
+        <Button @type={{bs-default @submitButtonType "primary"}} type="submit" disabled={{bs-default @submitDisabled false}}>{{@submitTitle}}</Button>
       {{else}}
         <Button @type="primary" @onClick={{@onClose}}>{{bs-default @closeTitle "Ok"}}</Button>
       {{/if}}

addon/components/bs-modal/footer.js

@@ -70,8 +70,14 @@ import { action } from '@ember/object';
 export default class ModalFooter extends Component {
   @action
   handleSubmit(event) {
-    // the footer is a <form>, so prevent the browser's native submission (and the page reload that comes with it)
-    event.preventDefault();
-    this.args.onSubmit?.();
+    if (!this.args.onSubmit) {
+      return;
+    }
+    // the footer is a <form>, so prevent the browser's native submission (and the page reload that comes with it),
+    // unless an `action` attribute asks for it
+    if (!event.target.hasAttribute('action')) {
+      event.preventDefault();
+    }
+    this.args.onSubmit();
   }
 }

tests/integration/components/bs-alert-test.js

@@ -141,6 +141,22 @@ module('Integration | Component | bs-alert', function (hooks) {
     assert.dom('*').hasText('', 'alert has no content');
   });
 
+  test('alert is initially hidden when visible=null', async function (assert) {
+    await render(hbs`<BsAlert @type="success" @fade={{false}} @visible={{null}}>Test</BsAlert>`);
+
+    assert.dom('.alert').doesNotExist('alert has no alert class');
+    assert.dom('*').hasText('', 'alert has no content');
+  });
+
+  test('alert can be hidden by setting visible=null', async function (assert) {
+    this.set('visible', true);
+    await render(hbs`<BsAlert @type="success" @fade={{false}} @visible={{this.visible}}>Test</BsAlert>`);
+    this.set('visible', null);
+    await settled();
+
+    assert.dom('.alert').doesNotExist('alert has no alert class');
+  });
+
   test('alert can be made visible when setting visible=true', async function (assert) {
     this.set('visible', false);
     await render(hbs`<BsAlert @type="success" @visible={{this.visible}} @fade={{false}}>Test</BsAlert>`);

tests/integration/components/bs-form-test.js

@@ -33,6 +33,10 @@ import setupNoDeprecations from '../../helpers/setup-no-deprecations';
 import a11yAudit from 'ember-a11y-testing/test-support/audit';
 import sinon from 'sinon';
 import Ember from 'ember';
+import { dependencySatisfies, macroCondition } from '@embroider/macros';
+
+// see https://github.com/kaliber5/ember-bootstrap/issues/1682
+const testUnlessEmbroider = macroCondition(dependencySatisfies('@embroider/core', '*')) ? skip : test;
 
 const nextRunloop = function () {
   return new Promise((resolve) => {
@@ -241,7 +245,7 @@ module('Integration | Component | bs-form', function (hooks) {
     assert.dom(`.${formFeedbackClass()}`).hasText('There is an error');
   });
 
-  skip('it does not catch errors thrown by onSubmit action', async function (assert) {
+  testUnlessEmbroider('it does not catch errors thrown by onSubmit action', async function (assert) {
     let onErrorStub = sinon.stub();
     let expectedError = new Error();
 

tests/integration/components/bs-modal/footer-test.js

@@ -1,3 +1,5 @@
+import { setComponentTemplate } from '@ember/component';
+import templateOnly from '@ember/component/template-only';
 import { module } from 'qunit';
 import { setupRenderingTest } from 'ember-qunit';
 import { click, render } from '@ember/test-helpers';
@@ -64,6 +66,57 @@ module('Integration | Component | bs-modal/footer', function (hooks) {
     assert.true(defaultPrevented, 'Native form submission is prevented.');
   });
 
+  module('native form submission', function (hooks) {
+    hooks.beforeEach(function () {
+      this.defaultPrevented = undefined;
+      this.listener = (event) => {
+        this.defaultPrevented = event.defaultPrevented;
+        // never let the test runner page navigate
+        event.preventDefault();
+      };
+      window.addEventListener('submit', this.listener);
+    });
+
+    hooks.afterEach(function () {
+      window.removeEventListener('submit', this.listener);
+    });
+
+    test('it is not prevented without onSubmit', async function (assert) {
+      await render(hbs`<BsModal::Footer @closeTitle="close" @submitTitle="submit" />`);
+      await click('.modal-footer button:last-child');
+
+      assert.false(this.defaultPrevented, 'Native form submission is not prevented.');
+    });
+
+    test('it is not prevented when the footer has an action attribute', async function (assert) {
+      let submitCount = 0;
+      this.set('submit', () => submitCount++);
+      await render(
+        hbs`<BsModal::Footer @closeTitle="close" @submitTitle="submit" @onSubmit={{this.submit}} action="/export" method="post" />`
+      );
+      await click('.modal-footer button:last-child');
+
+      assert.strictEqual(submitCount, 1, 'onSubmit is called once.');
+      assert.false(this.defaultPrevented, 'Native form submission is not prevented.');
+    });
+
+    test('a custom buttonComponent submits the footer', async function (assert) {
+      let submitCount = 0;
+      this.set('submit', () => submitCount++);
+      this.CustomButton = setComponentTemplate(
+        hbs`<button type="button" class="custom-button" ...attributes>{{yield}}</button>`,
+        templateOnly()
+      );
+      await render(
+        hbs`<BsModal::Footer @closeTitle="close" @submitTitle="submit" @onSubmit={{this.submit}} @buttonComponent={{this.CustomButton}} />`
+      );
+      await click('.modal-footer button:last-child');
+
+      assert.strictEqual(submitCount, 1, 'onSubmit is called once.');
+      assert.true(this.defaultPrevented, 'Native form submission is prevented.');
+    });
+  });
+
   test('Footer can have a custom submitButtonType', async function (assert) {
     await render(hbs`<BsModal::Footer @closeTitle="close" @submitTitle="submit" @submitButtonType="danger" />`);
     assert.dom('.modal-footer button:last-child').hasClass('btn-danger', 'Submit button is a danger button.');