diff options
author | GeoSot <geo.sotis@gmail.com> | 2021-06-28 16:34:47 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-06-28 16:34:47 +0300 |
commit | 70dd7f6ff51a0bd3b09b0de3640b0517b8b08f64 (patch) | |
tree | 5c72cbf572eb3b4dc8ba721ca2f166a64567f0cc /js | |
parent | 45d26de72817b295c5f94c8426354fd5b7d0a1f9 (diff) |
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 <xhmikosr@gmail.com>
Diffstat (limited to 'js')
-rw-r--r-- | js/src/alert.js | 69 | ||||
-rw-r--r-- | js/tests/unit/alert.spec.js | 21 |
2 files changed, 40 insertions, 50 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) - } - - // Private - - _getRootElement(element) { - return getElementFromSelector(element) || element.closest(`.${CLASS_NAME_ALERT}`) - } + this._element.classList.remove(CLASS_NAME_SHOW) - _triggerCloseEvent(element) { - return EventHandler.trigger(element, EVENT_CLOSE) + const isAnimated = this._element.classList.contains(CLASS_NAME_FADE) + this._queueCallback(() => this._destroyElement(), this._element, isAnimated) } - _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) + // Private + _destroyElement() { + this._element.remove() + EventHandler.trigger(this._element, EVENT_CLOSED) + this.dispose() } // Static @@ -88,20 +75,16 @@ class Alert extends BaseComponent { return this.each(function () { const data = Alert.getOrCreateInstance(this) - if (config === 'close') { - data[config](this) + if (typeof config !== 'string') { + return } - }) - } - static handleDismiss(alertInstance) { - return function (event) { - if (event) { - event.preventDefault() + if (data[config] === undefined || config.startsWith('_') || config === 'constructor') { + throw new TypeError(`No method named "${config}"`) } - alertInstance.close(this) - } + data[config](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 = '<div class="alert"></div>' - const alertEl = document.querySelector('.alert') + const getAlert = () => document.querySelector('.alert') + const alertEl = getAlert() const alert = new Alert(alertEl) - const endTest = () => { + alertEl.addEventListener('close.bs.alert', event => { + event.preventDefault() setTimeout(() => { - expect(alert._removeElement).not.toHaveBeenCalled() + expect(getAlert()).not.toBeNull() done() }, 10) - } - - spyOn(alert, '_removeElement') - - alertEl.addEventListener('close.bs.alert', event => { - event.preventDefault() - endTest() }) 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() }) |