Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Restore offset option for dropdown component #32443

Merged
merged 3 commits into from Jan 28, 2021
Merged

Restore offset option for dropdown component #32443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ce3c21a
Restore offset option for dropdown component
joke2k Dec 12, 2020
118824b
Restore `offset` option for tooltip/popover components
rohit2sharma95 Dec 16, 2020
522626f
Update docs for `offset` option
rohit2sharma95 Dec 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 22 additions & 2 deletions js/src/dropdown.js
View file
Open in desktop
Expand Up @@ -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],
joke2k marked this conversation as resolved.
Show resolved Hide resolved
flip: true,
boundary: 'clippingParents',
reference: 'toggle',
Expand All @@ -81,7 +81,7 @@ const Default = {
}

const DefaultType = {
offset: '(number|string|function)',
offset: '(array|string|function)',
joke2k marked this conversation as resolved.
Show resolved Hide resolved
flip: 'boolean',
boundary: '(string|element)',
reference: '(string|element|object)',
Expand Down Expand Up @@ -298,6 +298,20 @@ class Dropdown extends BaseComponent {
return this._element.closest(`.${CLASS_NAME_NAVBAR}`) !== null
}

_getOffset() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rohit2sharma95 this seems to be duplicated. Might be worth checking at a later time if we it makes sense to deduplicate this. Lower priority right now :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, should tackle it in a new PR later?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to deduplicate this into utils/index.js and I was wondering if we can do the same for _getPopperConfig...
but the function signature becoming a little bit weird:

const getPopperOffsets = (element, config) => {
  if (typeof offset === 'string') {
    return offset.split(',').map(val => Number.parseInt(val, 10))
  }

  if (typeof offset === 'function') {
    return popperData => offset(popperData, element)
  }

  return offset
}

const getPopperConfig = (element, placement, offset, modifiers, options) => {
  const defaultBsConfig = {
    placement: placement,
    modifiers: [...(modifiers || [])],
    ...(options || {})
  }

  if (offset) {
    defaultBsConfig.modifiers.push({
      name: 'offset',
      options: {
        offset: getPopperOffsets(element, offset)
      }
    })
  }

  return defaultBsConfig
}

the element is necessary because offset configured as function needs it for the second parameter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to do this now. We'll have plenty of time after this lands :)

const { offset } = this._config

if (typeof offset === 'string') {
return offset.split(',').map(val => Number.parseInt(val, 10))
}

if (typeof offset === 'function') {
return popperData => offset(popperData, this._element)
}

return offset
}

_getPopperConfig() {
const popperConfig = {
placement: this._getPlacement(),
Expand All @@ -313,6 +327,12 @@ class Dropdown extends BaseComponent {
options: {
fallbackPlacements: ['top', 'right', 'bottom', 'left']
}
},
{
name: 'offset',
options: {
offset: this._getOffset()
}
}]
}

Expand Down
22 changes: 22 additions & 0 deletions js/src/tooltip.js
View file
Open in desktop
Expand Up @@ -50,6 +50,7 @@ const DefaultType = {
html: 'boolean',
selector: '(string|boolean)',
placement: '(string|function)',
offset: '(array|string|function)',
container: '(string|element|boolean)',
fallbackPlacements: 'array',
boundary: '(string|element)',
Expand Down Expand Up @@ -80,6 +81,7 @@ const Default = {
html: false,
selector: false,
placement: 'top',
offset: [0, 0],
container: false,
fallbackPlacements: ['top', 'right', 'bottom', 'left'],
boundary: 'clippingParents',
Expand Down Expand Up @@ -473,6 +475,20 @@ class Tooltip extends BaseComponent {
return context
}

_getOffset() {
const { offset } = this.config

if (typeof offset === 'string') {
return offset.split(',').map(val => Number.parseInt(val, 10))
}

if (typeof offset === 'function') {
return popperData => offset(popperData, this._element)
}

return offset
}

_getPopperConfig(attachment) {
const defaultBsConfig = {
placement: attachment,
Expand All @@ -484,6 +500,12 @@ class Tooltip extends BaseComponent {
fallbackPlacements: this.config.fallbackPlacements
}
},
{
name: 'offset',
options: {
offset: this._getOffset()
}
},
{
name: 'preventOverflow',
options: {
Expand Down
48 changes: 48 additions & 0 deletions js/tests/unit/dropdown.spec.js
View file
Open in desktop
Expand Up @@ -54,6 +54,54 @@ describe('Dropdown', () => {
expect(dropdown.toggle).toHaveBeenCalled()
})

it('should create offset modifier correctly when offset option is a function', done => {
fixtureEl.innerHTML = [
'<div class="dropdown">',
' <button class="btn dropdown-toggle" data-bs-toggle="dropdown">Dropdown</button>',
' <div class="dropdown-menu">',
' <a class="dropdown-item" href="#">Secondary link</a>',
' </div>',
'</div>'
].join('')

const getOffset = jasmine.createSpy('getOffset').and.returnValue([10, 20])
const btnDropdown = fixtureEl.querySelector('[data-bs-toggle="dropdown"]')
const dropdown = new Dropdown(btnDropdown, {
offset: getOffset,
popperConfig: {
onFirstUpdate: state => {
expect(getOffset).toHaveBeenCalledWith({
popper: state.rects.popper,
reference: state.rects.reference,
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', () => {
fixtureEl.innerHTML = [
'<div class="dropdown">',
' <button class="btn dropdown-toggle" data-bs-toggle="dropdown" data-bs-offset="10,20">Dropdown</button>',
' <div class="dropdown-menu">',
' <a class="dropdown-item" href="#">Secondary link</a>',
' </div>',
'</div>'
].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 = [
'<div class="dropdown">',
Expand Down
35 changes: 35 additions & 0 deletions js/tests/unit/tooltip.spec.js
View file
Open in desktop
Expand Up @@ -107,6 +107,41 @@ describe('Tooltip', () => {
tooltipInContainerEl.click()
})

it('should create offset modifier when offset is passed as a function', done => {
fixtureEl.innerHTML = '<a href="#" rel="tooltip" title="Offset from function">'

const getOffset = jasmine.createSpy('getOffset').and.returnValue([10, 20])
const tooltipEl = fixtureEl.querySelector('a')
const tooltip = new Tooltip(tooltipEl, {
offset: getOffset,
popperConfig: {
onFirstUpdate: state => {
expect(getOffset).toHaveBeenCalledWith({
popper: state.rects.popper,
reference: state.rects.reference,
placement: state.placement
}, tooltipEl)
done()
}
}
})

const offset = tooltip._getOffset()

expect(typeof offset).toEqual('function')

tooltip.show()
})

it('should create offset modifier when offset option is passed in data attribute', () => {
fixtureEl.innerHTML = '<a href="#" rel="tooltip" data-bs-offset="10,20" title="Another tooltip">'

const tooltipEl = fixtureEl.querySelector('a')
const tooltip = new Tooltip(tooltipEl)

expect(tooltip._getOffset()).toEqual([10, 20])
})

it('should allow to pass config to Popper with `popperConfig`', () => {
fixtureEl.innerHTML = '<a href="#" rel="tooltip">'

Expand Down
10 changes: 10 additions & 0 deletions site/content/docs/5.0/components/dropdowns.md
View file
Open in desktop
Expand Up @@ -896,6 +896,16 @@ Options can be passed via data attributes or JavaScript. For data attributes, ap
<td><code>'dynamic'</code></td>
<td>By default, we use Popper for dynamic positioning. Disable this with <code>static</code>.</td>
</tr>
<tr>
<td><code>offset</code></td>
<td>array | string | function</td>
<td><code>[0, 0]</code></td>
<td>
<p>Offset of the dropdown relative to its target. You can pass a string in data attributes with comma separated values like: <code>data-bs-offset="10,20"</code></p>
<p>When a function is used to determine the offset, it is called with an object containing the popper placement, the reference, and popper rects as its first argument. The triggering element DOM node is passed as the second argument. The function must return an array with two numbers: <code>[<a href="https://popper.js.org/docs/v2/modifiers/offset/#skidding-1">skidding</a>, <a href="https://popper.js.org/docs/v2/modifiers/offset/#distance-1">distance</a>]</code>.</p>
<p>For more information refer to Popper's <a href="https://popper.js.org/docs/v2/modifiers/offset/#options">offset docs</a>.</p>
</td>
</tr>
<tr>
<td><code>popperConfig</code></td>
<td>null | object</td>
Expand Down
10 changes: 10 additions & 0 deletions site/content/docs/5.0/components/popovers.md
View file
Open in desktop
Expand Up @@ -267,6 +267,16 @@ Note that for security reasons the `sanitize`, `sanitizeFn`, and `allowList` opt
<td><code>null</code></td>
<td>Here you can supply your own sanitize function. This can be useful if you prefer to use a dedicated library to perform sanitization.</td>
</tr>
<tr>
<td><code>offset</code></td>
<td>array | string | function</td>
<td><code>[0, 0]</code></td>
<td>
<p>Offset of the popover relative to its target. You can pass a string in data attributes with comma separated values like: <code>data-bs-offset="10,20"</code></p>
<p>When a function is used to determine the offset, it is called with an object containing the popper placement, the reference, and popper rects as its first argument. The triggering element DOM node is passed as the second argument. The function must return an array with two numbers: <code>[<a href="https://popper.js.org/docs/v2/modifiers/offset/#skidding-1">skidding</a>, <a href="https://popper.js.org/docs/v2/modifiers/offset/#distance-1">distance</a>]</code>.</p>
<p>For more information refer to Popper's <a href="https://popper.js.org/docs/v2/modifiers/offset/#options">offset docs</a>.</p>
</td>
</tr>
<tr>
<td><code>popperConfig</code></td>
<td>null | object</td>
Expand Down
10 changes: 10 additions & 0 deletions site/content/docs/5.0/components/tooltips.md
View file
Open in desktop
Expand Up @@ -292,6 +292,16 @@ Note that for security reasons the `sanitize`, `sanitizeFn`, and `allowList` opt
<td><code>null</code></td>
<td>Here you can supply your own sanitize function. This can be useful if you prefer to use a dedicated library to perform sanitization.</td>
</tr>
<tr>
<td><code>offset</code></td>
<td>array | string | function</td>
<td><code>[0, 0]</code></td>
<td>
<p>Offset of the tooltip relative to its target. You can pass a string in data attributes with comma separated values like: <code>data-bs-offset="10,20"</code></p>
<p>When a function is used to determine the offset, it is called with an object containing the popper placement, the reference, and popper rects as its first argument. The triggering element DOM node is passed as the second argument. The function must return an array with two numbers: <code>[<a href="https://popper.js.org/docs/v2/modifiers/offset/#skidding-1">skidding</a>, <a href="https://popper.js.org/docs/v2/modifiers/offset/#distance-1">distance</a>]</code>.</p>
<p>For more information refer to Popper's <a href="https://popper.js.org/docs/v2/modifiers/offset/#options">offset docs</a>.</p>
</td>
</tr>
<tr>
<td><code>popperConfig</code></td>
<td>null | object</td>
Expand Down
1 change: 1 addition & 0 deletions site/content/docs/5.0/migration.md
View file
Open in desktop
Expand Up @@ -20,6 +20,7 @@ toc: true

### JavaScript

- Restored `offset` option for Dropdown, Popover and Tooltip plugins.
- The default value for the `fallbackPlacements` is changed to `['top', 'right', 'bottom', 'left']` for better placement of popper elements.

## v5.0.0-beta1
Expand Down