danielfarrell / bootstrap-combobox

A combobox plugin that works with twitter bootstrap
847 stars 327 forks source link

support Bootstrap 4 #263

Closed y-code closed 5 years ago

y-code commented 5 years ago

Currently how the combobox looks is not good at all with Bootstrap 4, where only caret icon appears on the right to the input, although the functions are fine.

It is because the migration from Bootstrap 3 to 4 and also the migration from Glyphicons are required. Dropdown uses Popper.js, so that bootstrap-combobox should work with it. Dropdown also works without Popper.js, so that I made it also work even without Popper.js.

I verified it works with the following versions of Bootstrap.

tiesont commented 5 years ago

Thank you. I'll try to spend some time reviewing this. If all looks good, seems like a good candidate for merging.

saiena commented 5 years ago

I also found that the "input-group-addon" class was removed in final bootstrap 4 and replaced with "input-group-append" and "input-group-prepend".

I therefore had to change the code to:

, template: function() {  
    if (this.options.bsVersion == '2') {  
        return '<div class="combobox-container"><input type="hidden" /> <div class="input-append"> <input type="text" autocomplete="off" /> <span class="add-on dropdown-toggle" data-dropdown="dropdown"> <span class="caret"/> <i class="icon-remove"/> </span> </div> </div>'  
    } else if (this.options.bsVersion == '4') {  
        return '<div class="combobox-container"> <input type="hidden" /> <div class="input-group"><input type="text" autocomplete="off" /> <div class="input-group-append"><span class="input-group-text dropdown-toggle" data-dropdown="dropdown"><span class="caret" /> <span class="glyphicon glyphicon-remove" /> </span> </div> </div> </div>'          
    } else {  
        return '<div class="combobox-container"> <input type="hidden" /> <div class="input-group"> <input type="text" autocomplete="off" /> <span class="input-group-addon dropdown-toggle" data-dropdown="dropdown"> <span class="caret" /> <span class="glyphicon glyphicon-remove" /> </span> </div> </div>'  
    }  
}

And the CSS code change:

.form-search .combobox-container .input-group-append,
.form-inline .combobox-container .input-group-append

The changed code also makes the entire appended caret container the menu toggle.

tiesont commented 5 years ago

Sorry, just getting around to checking this.

The one issue I think prevents me from pulling this in as-is would be the use of template literals. Internet Explorer doesn't support template literals, and I'm not 100% sold on dropping support for IE just yet. If you can make this ES5 compliant, I can pull it in. Otherwise, I would need to move this to a 2.0 branch, if we're going to stick with semver.

y-code commented 5 years ago

Sure. I'll fix that in a week or so.

y-code commented 5 years ago

@tiesont I made it ES5 compliant.

zrabin commented 4 years ago

@tiesont Could a new release be cut that includes this change? I'm trying to use the version available via CDN, but it's not up-to-date with this change (neither is the NPM version).

tiesont commented 4 years ago

@zrabin I don't think I have push rights on the npm package, at least at the moment. I'll see if I've been granted rights to create releases (seems like I do) here and will try to create a release if I can. Not sure about CDN - if it's CDNjs, there's a process involved there that isn't super fun if you're not an admin for the source project (which I am not).

Will probably be a day or so before I can get to any of these, but I'll respond as I know more.

zrabin commented 4 years ago

@tiesont Thanks for the prompt response. I currently have a local version of the master branch, which I'm referencing in my app, but would rather not use that long-term. Tagging @danielfarrell in case he can help out with setting up the release/CDN.

Thanks again!

tiesont commented 4 years ago

@zrabin What CDN are you referencing? If it's cdnjs, as long as the project structure hasn't changed (I don't think it has), I probably just need to raise an issue there asking for it to be updated (might see if I could enable auto-update while I'm at it). That was the gist of what I had to do for Bootbox, which is another project where I'm a maintainer / collaborator but not an admin.

It looks like I can create a release, so I'll see if I can find some time to do that (but like I mentioned, might be a day or two).

tiesont commented 4 years ago

@zrabin I've contacted Daniel about being added to the npm project, so we'll see how that goes.

tiesont commented 4 years ago

@zrabin Looks like Daniel doesn't do anything special for the releases, so I went ahead and created a 1.2.0 release (see https://github.com/danielfarrell/bootstrap-combobox/releases/tag/1.2.0). Probably worth creating a new issue for updating npm and cdnjs.

zrabin commented 4 years ago

@tiesont Thanks! I'll create a new issue regarding the NPM and CDNJS updates.

Edit: Just noticed that you already created an issue. Thanks!