From 9148528b91616534ed7271ec53f2fe83ba727be7 Mon Sep 17 00:00:00 2001 From: Nathan Walters Date: Wed, 7 Jun 2023 16:48:44 -0700 Subject: [PATCH 1/3] Fix this reference for functions --- js/src/dropdown.js | 2 +- js/src/tooltip.js | 2 +- js/src/util/index.js | 2 +- js/src/util/template-factory.js | 2 +- js/tests/unit/popover.spec.js | 27 +++++++++++++++++++++++++++ js/tests/unit/tooltip.spec.js | 13 +++++++++++++ js/tests/unit/util/index.spec.js | 4 ++-- 7 files changed, 46 insertions(+), 6 deletions(-) diff --git a/js/src/dropdown.js b/js/src/dropdown.js index af5fd16fc956..6381b91be4b0 100644 --- a/js/src/dropdown.js +++ b/js/src/dropdown.js @@ -320,7 +320,7 @@ class Dropdown extends BaseComponent { return { ...defaultBsPopperConfig, - ...execute(this._config.popperConfig, [defaultBsPopperConfig]) + ...execute(this._config.popperConfig, [undefined, defaultBsPopperConfig]) } } diff --git a/js/src/tooltip.js b/js/src/tooltip.js index 1252811573e9..a478bd2d8317 100644 --- a/js/src/tooltip.js +++ b/js/src/tooltip.js @@ -436,7 +436,7 @@ class Tooltip extends BaseComponent { return { ...defaultBsPopperConfig, - ...execute(this._config.popperConfig, [defaultBsPopperConfig]) + ...execute(this._config.popperConfig, [undefined, defaultBsPopperConfig]) } } diff --git a/js/src/util/index.js b/js/src/util/index.js index 68b8d8988195..220c43e1e5b0 100644 --- a/js/src/util/index.js +++ b/js/src/util/index.js @@ -223,7 +223,7 @@ const defineJQueryPlugin = plugin => { } const execute = (possibleCallback, args = [], defaultValue = possibleCallback) => { - return typeof possibleCallback === 'function' ? possibleCallback(...args) : defaultValue + return typeof possibleCallback === 'function' ? possibleCallback.call(...args) : defaultValue } const executeAfterTransition = (callback, transitionElement, waitForTransition = true) => { diff --git a/js/src/util/template-factory.js b/js/src/util/template-factory.js index f73589bcc785..6d1532b4d4bb 100644 --- a/js/src/util/template-factory.js +++ b/js/src/util/template-factory.js @@ -143,7 +143,7 @@ class TemplateFactory extends Config { } _resolvePossibleFunction(arg) { - return execute(arg, [this]) + return execute(arg, [undefined, this]) } _putElementInTemplate(element, templateElement) { diff --git a/js/tests/unit/popover.spec.js b/js/tests/unit/popover.spec.js index 53dc7d89ea6d..6b8195039115 100644 --- a/js/tests/unit/popover.spec.js +++ b/js/tests/unit/popover.spec.js @@ -95,6 +95,33 @@ describe('Popover', () => { }) }) + it('should call content and title functions with correct this value', () => { + return new Promise(resolve => { + fixtureEl.innerHTML = 'BS twitter' + + const popoverEl = fixtureEl.querySelector('a') + const popover = new Popover(popoverEl, { + title() { + return this.dataset.foo + }, + content() { + return this.dataset.foo + } + }) + + popoverEl.addEventListener('shown.bs.popover', () => { + const popoverDisplayed = document.querySelector('.popover') + + expect(popoverDisplayed).not.toBeNull() + expect(popoverDisplayed.querySelector('.popover-header').textContent).toEqual('bar') + expect(popoverDisplayed.querySelector('.popover-body').textContent).toEqual('bar') + resolve() + }) + + popover.show() + }) + }) + it('should show a popover with just content without having header', () => { return new Promise(resolve => { fixtureEl.innerHTML = 'Nice link' diff --git a/js/tests/unit/tooltip.spec.js b/js/tests/unit/tooltip.spec.js index 080432e9abab..1c15e52b72f4 100644 --- a/js/tests/unit/tooltip.spec.js +++ b/js/tests/unit/tooltip.spec.js @@ -1335,6 +1335,19 @@ describe('Tooltip', () => { expect(tooltip._getTitle()).toEqual('test') }) + + it('should call title function with correct this value', () => { + fixtureEl.innerHTML = '' + + const tooltipEl = fixtureEl.querySelector('a') + const tooltip = new Tooltip(tooltipEl, { + title() { + return this.dataset.foo + } + }) + + expect(tooltip._getTitle()).toEqual('bar') + }) }) describe('getInstance', () => { diff --git a/js/tests/unit/util/index.spec.js b/js/tests/unit/util/index.spec.js index 4065a9168032..9e154818f288 100644 --- a/js/tests/unit/util/index.spec.js +++ b/js/tests/unit/util/index.spec.js @@ -521,10 +521,10 @@ describe('Util', () => { it('should execute if arg is function & return the result', () => { const functionFoo = (num1, num2 = 10) => num1 + num2 - const resultFoo = Util.execute(functionFoo, [4, 5]) + const resultFoo = Util.execute(functionFoo, [undefined, 4, 5]) expect(resultFoo).toBe(9) - const resultFoo1 = Util.execute(functionFoo, [4]) + const resultFoo1 = Util.execute(functionFoo, [undefined, 4]) expect(resultFoo1).toBe(14) const functionBar = () => 'foo' From 03075be3039482fcd01ee5f2b1613b09dd0d624c Mon Sep 17 00:00:00 2001 From: Nathan Sarang-Walters Date: Wed, 19 Jun 2024 12:20:12 -0700 Subject: [PATCH 2/3] Improve unit tests for dropdown/tooltip popperConfig function --- js/tests/unit/dropdown.spec.js | 5 ++++- js/tests/unit/tooltip.spec.js | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/js/tests/unit/dropdown.spec.js b/js/tests/unit/dropdown.spec.js index 156005588dbe..63ae4bd102bc 100644 --- a/js/tests/unit/dropdown.spec.js +++ b/js/tests/unit/dropdown.spec.js @@ -172,7 +172,10 @@ describe('Dropdown', () => { const popperConfig = dropdown._getPopperConfig() - expect(getPopperConfig).toHaveBeenCalled() + // Ensure that the function was called with the default config. + expect(getPopperConfig).toHaveBeenCalledWith(jasmine.objectContaining({ + placement: jasmine.any(String) + })) expect(popperConfig.placement).toEqual('left') }) }) diff --git a/js/tests/unit/tooltip.spec.js b/js/tests/unit/tooltip.spec.js index 75fdc6a4aff4..d70cef639fd2 100644 --- a/js/tests/unit/tooltip.spec.js +++ b/js/tests/unit/tooltip.spec.js @@ -177,7 +177,10 @@ describe('Tooltip', () => { const popperConfig = tooltip._getPopperConfig('top') - expect(getPopperConfig).toHaveBeenCalled() + // Ensure that the function was called with the default config. + expect(getPopperConfig).toHaveBeenCalledWith(jasmine.objectContaining({ + placement: jasmine.any(String) + })) expect(popperConfig.placement).toEqual('left') }) From a1a57a0338901b20a56063c9a683d173632f2ac4 Mon Sep 17 00:00:00 2001 From: Nathan Sarang-Walters Date: Thu, 20 Jun 2024 10:45:12 -0700 Subject: [PATCH 3/3] Add/improve tests --- js/src/tooltip.js | 2 +- js/tests/unit/popover.spec.js | 27 +++++++++++++++++++++++++++ js/tests/unit/tooltip.spec.js | 22 +++++++++++++++++++--- 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/js/src/tooltip.js b/js/src/tooltip.js index 24ae2a111018..92d455349336 100644 --- a/js/src/tooltip.js +++ b/js/src/tooltip.js @@ -392,7 +392,7 @@ class Tooltip extends BaseComponent { } _resolvePossibleFunction(arg) { - return execute(arg, [this._element]) + return execute(arg, [this._element, this._element]) } _getPopperConfig(attachment) { diff --git a/js/tests/unit/popover.spec.js b/js/tests/unit/popover.spec.js index 6b8195039115..ba38ebe06629 100644 --- a/js/tests/unit/popover.spec.js +++ b/js/tests/unit/popover.spec.js @@ -95,6 +95,33 @@ describe('Popover', () => { }) }) + it('should call content and title functions with trigger element', () => { + return new Promise(resolve => { + fixtureEl.innerHTML = 'BS twitter' + + const popoverEl = fixtureEl.querySelector('a') + const popover = new Popover(popoverEl, { + title(el) { + return el.dataset.foo + }, + content(el) { + return el.dataset.foo + } + }) + + popoverEl.addEventListener('shown.bs.popover', () => { + const popoverDisplayed = document.querySelector('.popover') + + expect(popoverDisplayed).not.toBeNull() + expect(popoverDisplayed.querySelector('.popover-header').textContent).toEqual('bar') + expect(popoverDisplayed.querySelector('.popover-body').textContent).toEqual('bar') + resolve() + }) + + popover.show() + }) + }) + it('should call content and title functions with correct this value', () => { return new Promise(resolve => { fixtureEl.innerHTML = 'BS twitter' diff --git a/js/tests/unit/tooltip.spec.js b/js/tests/unit/tooltip.spec.js index d70cef639fd2..37f2c230d037 100644 --- a/js/tests/unit/tooltip.spec.js +++ b/js/tests/unit/tooltip.spec.js @@ -922,10 +922,12 @@ describe('Tooltip', () => { it('should show a tooltip with custom class provided as a function in config', () => { return new Promise(resolve => { - fixtureEl.innerHTML = '' + fixtureEl.innerHTML = '' - const spy = jasmine.createSpy('customClass').and.returnValue('custom-class') const tooltipEl = fixtureEl.querySelector('a') + const spy = jasmine.createSpy('customClass').and.callFake(function (el) { + return `${el.dataset.classA} ${this.dataset.classB}` + }) const tooltip = new Tooltip(tooltipEl, { customClass: spy }) @@ -934,7 +936,8 @@ describe('Tooltip', () => { const tip = document.querySelector('.tooltip') expect(tip).not.toBeNull() expect(spy).toHaveBeenCalled() - expect(tip).toHaveClass('custom-class') + expect(tip).toHaveClass('custom-class-a') + expect(tip).toHaveClass('custom-class-b') resolve() }) @@ -1341,6 +1344,19 @@ describe('Tooltip', () => { expect(tooltip._getTitle()).toEqual('test') }) + it('should call title function with trigger element', () => { + fixtureEl.innerHTML = '' + + const tooltipEl = fixtureEl.querySelector('a') + const tooltip = new Tooltip(tooltipEl, { + title(el) { + return el.dataset.foo + } + }) + + expect(tooltip._getTitle()).toEqual('bar') + }) + it('should call title function with correct this value', () => { fixtureEl.innerHTML = ''