From 5c8890687aa631a315438529346e89f4943d82a8 Mon Sep 17 00:00:00 2001 From: joke2k Date: Sat, 12 Dec 2020 00:54:29 +0000 Subject: [PATCH 1/6] Restore offset option for dropdown component --- js/src/dropdown.js | 31 +++++++++++++++++++ site/content/docs/5.0/components/dropdowns.md | 10 ++++++ 2 files changed, 41 insertions(+) diff --git a/js/src/dropdown.js b/js/src/dropdown.js index 4d65008f8..8824935d7 100644 --- a/js/src/dropdown.js +++ b/js/src/dropdown.js @@ -289,6 +289,31 @@ class Dropdown extends BaseComponent { return this._element.closest(`.${CLASS_NAME_NAVBAR}`) !== null } + _getOffset() { + let offset = [0, 0] + + if (!this._config.offset) { + return offset + } + + if (typeof this._config.offset === 'number') { + offset[0] = this._config.offset + } else if (typeof this._config.offset === 'string') { + offset = offset.split(',') + if (offset.length === 1) { + offset = [offset[0], 0] + } + + offset = offset.map(val => Number.parseInt(val, 10)) + } else if (typeof this._config.offset === 'function') { + offset = (popper, reference, placement) => { + return offset({ popper, reference, placement }, this._element) + } + } + + return offset + } + _getPopperConfig() { const popperConfig = { placement: this._getPlacement(), @@ -304,6 +329,12 @@ class Dropdown extends BaseComponent { options: { fallbackPlacements: ['top', 'right', 'bottom', 'left'] } + }, + { + name: 'offset', + options: { + offset: this._getOffset() + } }] } diff --git a/site/content/docs/5.0/components/dropdowns.md b/site/content/docs/5.0/components/dropdowns.md index 50dcde2ba..416b80fd0 100644 --- a/site/content/docs/5.0/components/dropdowns.md +++ b/site/content/docs/5.0/components/dropdowns.md @@ -896,6 +896,16 @@ Options can be passed via data attributes or JavaScript. For data attributes, ap 'dynamic' By default, we use Popper for dynamic positioning. Disable this with static. + + offset + number | string | function + 0 + +

Offset of the dropdown relative to its target.

+

When a function is used to determine the offset, it is called with an object containing the popper offsets object data as its first argument. The function must return an array with two numbers: [skidding, distance]. The triggering element DOM node is passed as the second argument.

+

For more information refer to Popper.js's offset docs.

+ + popperConfig null | object From 5d22c10d081c3fb77221283715d49f59c8ee17c3 Mon Sep 17 00:00:00 2001 From: joke2k Date: Sat, 12 Dec 2020 01:27:54 +0000 Subject: [PATCH 2/6] tests for dropdown offsets --- js/src/dropdown.js | 14 ++++---- js/tests/unit/dropdown.spec.js | 36 +++++++++++++++++++ site/content/docs/5.0/components/dropdowns.md | 2 +- 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/js/src/dropdown.js b/js/src/dropdown.js index 8824935d7..177c7f7d5 100644 --- a/js/src/dropdown.js +++ b/js/src/dropdown.js @@ -290,22 +290,22 @@ class Dropdown extends BaseComponent { } _getOffset() { - let offset = [0, 0] + let { offset } = this._config - if (!this._config.offset) { - return offset + if (!offset) { + return [0, 0] } - if (typeof this._config.offset === 'number') { - offset[0] = this._config.offset - } else if (typeof this._config.offset === 'string') { + if (typeof offset === 'number') { + offset = [offset, 0] + } else if (typeof offset === 'string') { offset = offset.split(',') if (offset.length === 1) { offset = [offset[0], 0] } offset = offset.map(val => Number.parseInt(val, 10)) - } else if (typeof this._config.offset === 'function') { + } else if (typeof offset === 'function') { offset = (popper, reference, placement) => { return offset({ popper, reference, placement }, this._element) } diff --git a/js/tests/unit/dropdown.spec.js b/js/tests/unit/dropdown.spec.js index d2171f369..c59cd3b02 100644 --- a/js/tests/unit/dropdown.spec.js +++ b/js/tests/unit/dropdown.spec.js @@ -54,6 +54,42 @@ describe('Dropdown', () => { expect(dropdown.toggle).toHaveBeenCalled() }) + it('should create offset modifier correctly when offset option is a function', () => { + fixtureEl.innerHTML = [ + '
', + ' ', + '
', + ' Secondary link', + '
', + '
' + ].join('') + + const getOffset = () => [10, 20] + const btnDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]') + const dropdown = new Dropdown(btnDropdown, { + offset: getOffset + }) + const offset = dropdown._getOffset() + + expect(typeof offset).toEqual('function') + }) + + it('should create offset modifier correctly when offset option is a string into data attribute', () => { + fixtureEl.innerHTML = [ + '
', + ' ', + '
', + ' Secondary link', + '
', + '
' + ].join('') + + const btnDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]') + const dropdown = new Dropdown(btnDropdown) + + expect(dropdown._getOffset()).toEqual([10, 20]) + }) + it('should allow to pass config to Popper with `popperConfig`', () => { fixtureEl.innerHTML = [ '
', diff --git a/site/content/docs/5.0/components/dropdowns.md b/site/content/docs/5.0/components/dropdowns.md index 416b80fd0..948eedeb8 100644 --- a/site/content/docs/5.0/components/dropdowns.md +++ b/site/content/docs/5.0/components/dropdowns.md @@ -899,7 +899,7 @@ Options can be passed via data attributes or JavaScript. For data attributes, ap offset number | string | function - 0 + "0,0"

Offset of the dropdown relative to its target.

When a function is used to determine the offset, it is called with an object containing the popper offsets object data as its first argument. The function must return an array with two numbers: [skidding, distance]. The triggering element DOM node is passed as the second argument.

From 2090329d546cbce97c8f2756bfa0cc6781a7f5f5 Mon Sep 17 00:00:00 2001 From: joke2k Date: Sun, 13 Dec 2020 11:31:49 +0000 Subject: [PATCH 3/6] simplify offset config --- js/src/dropdown.js | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/js/src/dropdown.js b/js/src/dropdown.js index 177c7f7d5..78ad80371 100644 --- a/js/src/dropdown.js +++ b/js/src/dropdown.js @@ -72,7 +72,7 @@ const PLACEMENT_RIGHT = isRTL ? 'left-start' : 'right-start' const PLACEMENT_LEFT = isRTL ? 'right-start' : 'left-start' const Default = { - offset: 0, + offset: [0, 0], flip: true, boundary: 'clippingParents', reference: 'toggle', @@ -81,7 +81,7 @@ const Default = { } const DefaultType = { - offset: '(number|string|function)', + offset: '(array|string|function)', flip: 'boolean', boundary: '(string|element)', reference: '(string|element)', @@ -296,17 +296,10 @@ class Dropdown extends BaseComponent { return [0, 0] } - if (typeof offset === 'number') { - offset = [offset, 0] - } else if (typeof offset === 'string') { - offset = offset.split(',') - if (offset.length === 1) { - offset = [offset[0], 0] - } - - offset = offset.map(val => Number.parseInt(val, 10)) + if (typeof offset === 'string') { + offset = offset.split(',').map(val => Number.parseInt(val, 10)) } else if (typeof offset === 'function') { - offset = (popper, reference, placement) => { + offset = ({ popper, reference, placement }) => { return offset({ popper, reference, placement }, this._element) } } From 7ab1dfea03815d1863d4a991a8fcc387a7725c15 Mon Sep 17 00:00:00 2001 From: joke2k Date: Sun, 13 Dec 2020 15:56:10 +0000 Subject: [PATCH 4/6] fix getOffset with function with test --- js/src/dropdown.js | 16 ++++++---------- js/tests/unit/dropdown.spec.js | 14 +++++++++++--- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/js/src/dropdown.js b/js/src/dropdown.js index 78ad80371..bd93b2bb0 100644 --- a/js/src/dropdown.js +++ b/js/src/dropdown.js @@ -290,18 +290,14 @@ class Dropdown extends BaseComponent { } _getOffset() { - let { offset } = this._config - - if (!offset) { - return [0, 0] - } + const { offset } = this._config if (typeof offset === 'string') { - offset = offset.split(',').map(val => Number.parseInt(val, 10)) - } else if (typeof offset === 'function') { - offset = ({ popper, reference, placement }) => { - return offset({ popper, reference, placement }, this._element) - } + return offset.split(',').map(val => Number.parseInt(val, 10)) + } + + if (typeof offset === 'function') { + return popperData => offset(popperData, this._element) } return offset diff --git a/js/tests/unit/dropdown.spec.js b/js/tests/unit/dropdown.spec.js index c59cd3b02..0b7e1621f 100644 --- a/js/tests/unit/dropdown.spec.js +++ b/js/tests/unit/dropdown.spec.js @@ -54,7 +54,7 @@ describe('Dropdown', () => { expect(dropdown.toggle).toHaveBeenCalled() }) - it('should create offset modifier correctly when offset option is a function', () => { + it('should create offset modifier correctly when offset option is a function', done => { fixtureEl.innerHTML = [ '
', ' ', @@ -64,14 +64,22 @@ describe('Dropdown', () => { '
' ].join('') - const getOffset = () => [10, 20] + const getOffset = jasmine.createSpy('getOffset').and.returnValue([10, 20]) const btnDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]') const dropdown = new Dropdown(btnDropdown, { - offset: getOffset + offset: getOffset, + popperConfig: { + onFirstUpdate: state => { + expect(getOffset).toHaveBeenCalledWith({ ...state.rects, placement: state.placement }, btnDropdown) + done() + } + } }) const offset = dropdown._getOffset() expect(typeof offset).toEqual('function') + + dropdown.show() }) it('should create offset modifier correctly when offset option is a string into data attribute', () => { From 7270c8ddc3cc3794ae7fe69acddce62adaab16f6 Mon Sep 17 00:00:00 2001 From: joke2k Date: Sun, 13 Dec 2020 19:46:22 +0000 Subject: [PATCH 5/6] fix dropdown offset docs --- js/tests/unit/dropdown.spec.js | 6 +++++- site/content/docs/5.0/components/dropdowns.md | 8 ++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/js/tests/unit/dropdown.spec.js b/js/tests/unit/dropdown.spec.js index 0b7e1621f..628b09d2d 100644 --- a/js/tests/unit/dropdown.spec.js +++ b/js/tests/unit/dropdown.spec.js @@ -70,7 +70,11 @@ describe('Dropdown', () => { offset: getOffset, popperConfig: { onFirstUpdate: state => { - expect(getOffset).toHaveBeenCalledWith({ ...state.rects, placement: state.placement }, btnDropdown) + expect(getOffset).toHaveBeenCalledWith({ + popper: state.rects.popper, + reference: state.rects.reference, + placement: state.placement + }, btnDropdown) done() } } diff --git a/site/content/docs/5.0/components/dropdowns.md b/site/content/docs/5.0/components/dropdowns.md index 948eedeb8..0aee28b32 100644 --- a/site/content/docs/5.0/components/dropdowns.md +++ b/site/content/docs/5.0/components/dropdowns.md @@ -898,12 +898,12 @@ Options can be passed via data attributes or JavaScript. For data attributes, ap offset - number | string | function - "0,0" + array | string | function + [0, 0]

Offset of the dropdown relative to its target.

-

When a function is used to determine the offset, it is called with an object containing the popper offsets object data as its first argument. The function must return an array with two numbers: [skidding, distance]. The triggering element DOM node is passed as the second argument.

-

For more information refer to Popper.js's offset docs.

+

When a function is used to determine the offset, it is called with an object containing the popper instance, the refecence Element and the placement as its first argument. The function must return an array with two numbers: [skidding, distance]. The triggering element DOM node is passed as the second argument.

+

For more information refer to Popper.js's offset docs.

From 98fe781fa5d8693ec8d8eacf44b9b74a8f5a1a10 Mon Sep 17 00:00:00 2001 From: Rohit Sharma Date: Wed, 16 Dec 2020 20:39:24 +0530 Subject: [PATCH 6/6] WIP: Fix the Popper warning issue - Set the default margin to 0 for dropdowns - Set the required offset in dropdowns --- js/src/dropdown.js | 2 +- scss/_dropdown.scss | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/js/src/dropdown.js b/js/src/dropdown.js index bd93b2bb0..9e9fba6b1 100644 --- a/js/src/dropdown.js +++ b/js/src/dropdown.js @@ -72,7 +72,7 @@ const PLACEMENT_RIGHT = isRTL ? 'left-start' : 'right-start' const PLACEMENT_LEFT = isRTL ? 'right-start' : 'left-start' const Default = { - offset: [0, 0], + offset: [0, 2], flip: true, boundary: 'clippingParents', reference: 'toggle', diff --git a/scss/_dropdown.scss b/scss/_dropdown.scss index 594c06399..6941c9e8d 100644 --- a/scss/_dropdown.scss +++ b/scss/_dropdown.scss @@ -22,7 +22,7 @@ display: none; // none by default, but block on "open" of the menu min-width: $dropdown-min-width; padding: $dropdown-padding-y $dropdown-padding-x; - margin: $dropdown-spacer 0 0; // override default ul + margin: 0; // Let Popper handle the space by offset @include font-size($dropdown-font-size); color: $dropdown-color; text-align: left; // Ensures proper alignment if parent has it changed (e.g., modal footer) @@ -174,6 +174,11 @@ .dropdown-menu.show { display: block; + + &:not([data-popper-placement]) { + margin: $dropdown-spacer 0 0; // override default ul for dropdown not using Popper + } + } // Dropdown section headers