From 70dd7f6ff51a0bd3b09b0de3640b0517b8b08f64 Mon Sep 17 00:00:00 2001 From: GeoSot Date: Mon, 28 Jun 2021 16:34:47 +0300 Subject: [PATCH] Changes to Alert component to match the others (#33402) Alert.js: Refactor code to match the other components * Use this._element * Remove handleDismiss method and keep its functionality on event * Change JqueryInterface to be more generic * Correct docs to be aligned with code, and add undocumented functionality * Update alerts.md Co-authored-by: XhmikosR --- js/src/alert.js | 73 ++++++++++------------ js/tests/unit/alert.spec.js | 23 +++---- site/content/docs/5.0/components/alerts.md | 32 +++++----- 3 files changed, 61 insertions(+), 67 deletions(-) diff --git a/js/src/alert.js b/js/src/alert.js index 75dbec71bc..0bbe62af59 100644 --- a/js/src/alert.js +++ b/js/src/alert.js @@ -7,7 +7,8 @@ import { defineJQueryPlugin, - getElementFromSelector + getElementFromSelector, + isDisabled } from './util/index' import EventHandler from './dom/event-handler' import BaseComponent from './base-component' @@ -48,38 +49,24 @@ class Alert extends BaseComponent { // Public - close(element) { - const rootElement = element ? this._getRootElement(element) : this._element - const customEvent = this._triggerCloseEvent(rootElement) + close() { + const closeEvent = EventHandler.trigger(this._element, EVENT_CLOSE) - if (customEvent === null || customEvent.defaultPrevented) { + if (closeEvent.defaultPrevented) { return } - this._removeElement(rootElement) + this._element.classList.remove(CLASS_NAME_SHOW) + + const isAnimated = this._element.classList.contains(CLASS_NAME_FADE) + this._queueCallback(() => this._destroyElement(), this._element, isAnimated) } // Private - - _getRootElement(element) { - return getElementFromSelector(element) || element.closest(`.${CLASS_NAME_ALERT}`) - } - - _triggerCloseEvent(element) { - return EventHandler.trigger(element, EVENT_CLOSE) - } - - _removeElement(element) { - element.classList.remove(CLASS_NAME_SHOW) - - const isAnimated = element.classList.contains(CLASS_NAME_FADE) - this._queueCallback(() => this._destroyElement(element), element, isAnimated) - } - - _destroyElement(element) { - element.remove() - - EventHandler.trigger(element, EVENT_CLOSED) + _destroyElement() { + this._element.remove() + EventHandler.trigger(this._element, EVENT_CLOSED) + this.dispose() } // Static @@ -88,21 +75,17 @@ class Alert extends BaseComponent { return this.each(function () { const data = Alert.getOrCreateInstance(this) - if (config === 'close') { - data[config](this) + if (typeof config !== 'string') { + return } + + if (data[config] === undefined || config.startsWith('_') || config === 'constructor') { + throw new TypeError(`No method named "${config}"`) + } + + data[config](this) }) } - - static handleDismiss(alertInstance) { - return function (event) { - if (event) { - event.preventDefault() - } - - alertInstance.close(this) - } - } } /** @@ -111,7 +94,19 @@ class Alert extends BaseComponent { * ------------------------------------------------------------------------ */ -EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DISMISS, Alert.handleDismiss(new Alert())) +EventHandler.on(document, EVENT_CLICK_DATA_API, SELECTOR_DISMISS, function (event) { + if (['A', 'AREA'].includes(this.tagName)) { + event.preventDefault() + } + + if (isDisabled(this)) { + return + } + + const target = getElementFromSelector(this) || this.closest(`.${CLASS_NAME_ALERT}`) + const alert = Alert.getOrCreateInstance(target) + alert.close() +}) /** * ------------------------------------------------------------------------ diff --git a/js/tests/unit/alert.spec.js b/js/tests/unit/alert.spec.js index 53dc0700c9..72cd23d89e 100644 --- a/js/tests/unit/alert.spec.js +++ b/js/tests/unit/alert.spec.js @@ -2,7 +2,7 @@ import Alert from '../../src/alert' import { getTransitionDurationFromElement } from '../../src/util/index' /** Test helpers */ -import { getFixture, clearFixture, jQueryMock } from '../helpers/fixture' +import { clearFixture, getFixture, jQueryMock } from '../helpers/fixture' describe('Alert', () => { let fixtureEl @@ -102,25 +102,20 @@ describe('Alert', () => { it('should not remove alert if close event is prevented', done => { fixtureEl.innerHTML = '
' - const alertEl = document.querySelector('.alert') + const getAlert = () => document.querySelector('.alert') + const alertEl = getAlert() const alert = new Alert(alertEl) - const endTest = () => { - setTimeout(() => { - expect(alert._removeElement).not.toHaveBeenCalled() - done() - }, 10) - } - - spyOn(alert, '_removeElement') - alertEl.addEventListener('close.bs.alert', event => { event.preventDefault() - endTest() + setTimeout(() => { + expect(getAlert()).not.toBeNull() + done() + }, 10) }) alertEl.addEventListener('closed.bs.alert', () => { - endTest() + throw new Error('should not fire closed event') }) alert.close() @@ -167,9 +162,9 @@ describe('Alert', () => { jQueryMock.fn.alert = Alert.jQueryInterface jQueryMock.elements = [alertEl] + expect(Alert.getInstance(alertEl)).toBeNull() jQueryMock.fn.alert.call(jQueryMock, 'close') - expect(Alert.getInstance(alertEl)).not.toBeNull() expect(fixtureEl.querySelector('.alert')).toBeNull() }) diff --git a/site/content/docs/5.0/components/alerts.md b/site/content/docs/5.0/components/alerts.md index a7e52f5f89..51feb966ed 100644 --- a/site/content/docs/5.0/components/alerts.md +++ b/site/content/docs/5.0/components/alerts.md @@ -147,35 +147,39 @@ Loop that generates the modifier classes with the `alert-variant()` mixin. ## JavaScript behavior -### Triggers +### Initialize -Enable dismissal of an alert via JavaScript: +Initialize elements as alerts ```js var alertList = document.querySelectorAll('.alert') -alertList.forEach(function (alert) { - new bootstrap.Alert(alert) +var alerts = [].slice.call(alertList).map(function (element) { + return new bootstrap.Alert(element) }) ``` +{{< callout info >}} +For the sole purpose of dismissing an alert, it isn't necessary to initialize the component manually via the JS API. By making use of `data-bs-dismiss="alert"`, the component will be initialized automatically and properly dismissed. -Or with `data` attributes on a button **within the alert**, as demonstrated above: +See the [triggers](#triggers) section for more details. +{{< /callout >}} + +### Triggers + +Dismissal can be achieved with `data` attributes on a button **within the alert** as demonstrated above: ```html ``` -Note that closing an alert will remove it from the DOM. +or on a button **outside the alert** using the `data-bs-target` as demonstrated above: -### Methods - -You can create an alert instance with the alert constructor, for example: - -```js -var myAlert = document.getElementById('myAlert') -var bsAlert = new bootstrap.Alert(myAlert) +```html + ``` -This makes an alert listen for click events on descendant elements which have the `data-bs-dismiss="alert"` attribute. (Not necessary when using the data-api's auto-initialization.) +**Note that closing an alert will remove it from the DOM.** + +### Methods