Skip to content

Commit

Permalink
move createLi into a setTimeout on init to prevent interference with …
Browse files Browse the repository at this point in the history
…page load (#1665, #2187)
  • Loading branch information
caseyjhol committed Mar 6, 2019
1 parent bce0441 commit 50b2327
Showing 1 changed file with 35 additions and 22 deletions.
57 changes: 35 additions & 22 deletions js/bootstrap-select.js
Original file line number Diff line number Diff line change
Expand Up @@ -768,11 +768,12 @@

this.multiple = this.$element.prop('multiple');
this.autofocus = this.$element.prop('autofocus');

this.$newElement = this.createDropdown();
this.createLi();
this.$element
.after(this.$newElement)
.prependTo(this.$newElement);

this.$button = this.$newElement.children('button');
this.$menu = this.$newElement.children(Selector.MENU);
this.$menuInner = this.$menu.children('.inner');
Expand Down Expand Up @@ -852,6 +853,7 @@
}

setTimeout(function () {
that.createLi();
that.$element.trigger('loaded' + EVENT_KEY);
});
},
Expand Down Expand Up @@ -1156,31 +1158,15 @@
});
},

createLi: function () {
var that = this,
iconBase = that.options.iconBase,
mainElements = [],
widestOption,
widestOptionLength = 0,
mainData = [],
optID = 0,
headerIndex = 0,
liIndex = -1; // increment liIndex whenever a new <li> element is created to ensure newIndex is correct

if (!this.selectpicker.view.titleOption) this.selectpicker.view.titleOption = document.createElement('option');

var checkMark;

if (that.options.showTick || that.multiple) {
checkMark = elementTemplates.span.cloneNode(false);
checkMark.className = iconBase + ' ' + that.options.tickIcon + ' check-mark';
elementTemplates.a.appendChild(checkMark);
}
setPlaceholder: function () {
var updateIndex = false;

if (this.options.title && !this.multiple) {
if (!this.selectpicker.view.titleOption) this.selectpicker.view.titleOption = document.createElement('option');

// this option doesn't create a new <li> element, but does add a new option, so liIndex is decreased
// since newIndex is recalculated on every refresh, liIndex needs to be decreased even if the titleOption is already appended
liIndex--;
updateIndex = true;

var element = this.$element[0],
isSelected = false,
Expand Down Expand Up @@ -1208,6 +1194,30 @@
if (isSelected) element.selectedIndex = 0;
}

return updateIndex;
},

createLi: function () {
var that = this,
iconBase = that.options.iconBase,
mainElements = [],
widestOption,
widestOptionLength = 0,
mainData = [],
optID = 0,
headerIndex = 0,
liIndex = -1; // increment liIndex whenever a new <li> element is created to ensure newIndex is correct

var checkMark;

if (that.options.showTick || that.multiple) {
checkMark = elementTemplates.span.cloneNode(false);
checkMark.className = iconBase + ' ' + that.options.tickIcon + ' check-mark';
elementTemplates.a.appendChild(checkMark);
}

if (this.setPlaceholder()) liIndex--;

var selectOptions = this.$element[0].options;

for (var index = 0, len = selectOptions.length; index < len; index++) {
Expand Down Expand Up @@ -1449,6 +1459,9 @@
},

render: function () {
// ensure titleOption is appended and selected (if necessary) before getting selectedOptions
this.setPlaceholder();

var that = this,
selectedOptions = this.$element[0].selectedOptions,
selectedCount = selectedOptions.length,
Expand Down

3 comments on commit 50b2327

@jaime-blazquez
Copy link

Choose a reason for hiding this comment

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

Hi,

This change broke some code we had, in case you operate on the select picker inmediately after creation, like in:

$('#testsp').selectpicker();
$('#testsp').selectpicker('deselectAll')

It doesn't work anymore, fails at bootstrap-select.js:2590, at changeAll, because current.elements doesn't exist. (first assignment of current to main happens at createLi.

As I've seen there are some other methods that use current.elements and we have a large code base, it would be acceptable to include some option to init the component inmediately? I'm interested in updating because of virtualScroll, but I'm afraid of subtle hidden bugs like the one we found.

@caseyjhol
Copy link
Member Author

Choose a reason for hiding this comment

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

@jaime-blazquez Please open a new bug report following our contribution guidelines and I'll investigate further. Thanks!

@caseyjhol
Copy link
Member Author

Choose a reason for hiding this comment

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

@jaime-blazquez Please 👍 and follow #2337.

Please sign in to comment.