select2 / select2-bootstrap-theme

A Select2 v4 Theme for Bootstrap 3
https://select2.github.io/select2-bootstrap-theme/
MIT License
871 stars 499 forks source link

Containment of select2 on a few examples #2

Closed ryanbeymer closed 9 years ago

ryanbeymer commented 9 years ago

@fk I know i'm a little late to the game but just wanting to make sure this is an issue.

A few select2's in your demonstrations blows outside the form-group

fk commented 9 years ago

Hi Ryan! You're of course right, this still is an issue – "still" because this has been happening since the Select2 v3/bootstrap3 version of select2-bootstrap-css.

First, just to make sure that we're talking about the same thing – could you confirm that this doesn't happen on initial page load, but only when you resize the browser window to be less wide than on initial page load?

If you compare the Select2 v3 demos to the Select2 v4 "theme" demos, you'll note that the Select2's that you mention behave almost exactly the same after you resize the window in the way previously described; if you consider the layout changes that I did for the Select2 v4 demos – removing the padding from html.demo – they might just behave exactly the same.

Furthermore, it's not only the "Select2 multi append/prepend" Select2's that behave badly, really all Multi Select2's do so, it's just not visible – here's a screenshot after resizing the window of the Select2 v4 theme demo page:

select2_multiple_after_resize

(please ignore the error, it's just a Chrome Extension and its jquery.min.map)

What's happening is that the inline style defining the width of .select2-search__field doesn't change on window.onresize

Without going into much details, maybe for starters @kevin-brown can confirm if the demo pages for Select2 v4 are set up correctly (using width: off); I frankly never looked too much into Select2's sizing options and don't want to go overboard in explaining what I understand after reading on them a bit just now.

@kevin-brown while I'm bothering you already ;-) – I quickly looked at the v4 Select2 examples and seem to note a difference in the https://select2.github.io/examples.html#responsive example's initial width settings – em vs px? Sorry for being too tired/ignorant to look at the code myself right now.

ryanbeymer commented 9 years ago

whoa wizbang stuff happening with github, didn't know it did that ;)

... anyways @fk thoughts on this

Obviously, less needs it too but wanted you thoughts on it, maybe I'm missing something here.

kevin-brown commented 9 years ago

Without going into much details, maybe for starters @kevin-brown can confirm if the demo pages for Select2 v4 are set up correctly (using width: off);

Setting width: 'off' isn't actually valid, but it's handled the same way as width: 'auto' (which is what you are looking for) by the browser. This is because the value of width is later set to the element as style="width: [width_option]", and off isn't a valid value for CSS width.

I'd recommend using width: 'auto' to be more explicit about that.

I quickly looked at the v4 Select2 examples and seem to note a difference in the https://select2.github.io/examples.html#responsive example's initial width settings – em vs px?

These should be handled the same way, as we just parse them all out in a single regex. All of our examples use % because it's easier to understand (and see the change), but I've tested with em before and it should still work the same.

What's happening is that the inline style defining the width of .select2-search__field doesn't change on window.onresize

To be more specific, we aren't triggering resizeSearch when the browser window is resized.

fk commented 9 years ago

@kevin-brown Thanks for the prompt reply and the heads up regarding width: 'off' – I must have taken this from the 3.5.2 docs (where it still was a valid option).

I just switched to width: 'auto' only to realize that the inline style now would overwrite the width: 100% I set via CSS – d'oh! ;-) While setting width: '100%' works fine, I find that a bit redundant. Is it too much to ask why you decided to remove the "off" option?

kevin-brown commented 9 years ago

Is it too much to ask why you decided to remove the "off" option?

It was removed because there was some way to work around it (which I never documented, I guess). I also completely forgot that it used to be valid, which explains why it was being used here.

I believe width: null should work the same, in that it won't set the width on the containers at all. It's what width: 'off' used to be equivalent to.

fk commented 9 years ago

@ryanbeymer

whoa wizbang stuff happening with github, didn't know it did that ;)

Can't follow you there, sorry. ;)

As for the code you linked, I cleaned things up a bit plus – since this is only happening in .input-group – altered the styles you provided to target only those. I recall the fix you provided was mentioned as a fix for https://github.com/t0m/select2-bootstrap-css/issues/42, too – and my first tests (still in latest Chrome only) confirm that. Let's hope that things also work in the rest of the target browsers – maybe you could help with that?

Thanks a bunch! :+1:

ryanbeymer commented 9 years ago

@fk I was just commenting on the automatic reference to the commit.

As far as the other browsers, what's the scope?

fk commented 9 years ago

I was just commenting on the automatic reference to the commit.

Ahh, I see … ;) Yep, there's a lot of nice stuff going on here. Gotta <3 GitHub!

As far as the other browsers, what's the scope?

Bootstrap 3 is IE8 and above plus the usual suspects (the evergreen browsers); Select2 is another question – I think it should be the same, but then again … "Paging Dr. @kevin-brown" ;) For https://github.com/t0m/select2-bootstrap-css/tree/bootstrap3 I tested "Chrome, Safari, Firefox, Opera (Mac) and IE8-IE10" – add IE11 and we should be good IMO.

fk commented 9 years ago

I believe width: null should work the same

@kevin-brown It does indeed, thanks for clarifying!

ryanbeymer commented 9 years ago

@fk is it even needed?

fk commented 9 years ago

Yep, otherwise the default width: 'resolve' kicks in which adds an inline style setting the width of .select2-container.

kevin-brown commented 9 years ago

I think it should be the same

It's pretty much the same as Bootstrap.

psalmody commented 9 years ago

Setting

$.fn.select2.defaults.set("width", "style");

before initializing .select2() fixed it for me!