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.

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

'auto' width not working #2045

Closed
ghost opened this issue Aug 21, 2018 · 12 comments
Closed

'auto' width not working #2045

ghost opened this issue Aug 21, 2018 · 12 comments
Labels

Comments

@ghost
Copy link

ghost commented Aug 21, 2018

@bazineta commented on May 3, 2018, 3:22 PM UTC:

While 'auto' width in the past has worked properly, resulting in a selector width equal to that of the widest option, it's not working now. This is visible in the examples in the documentation, where instead of being as wide as it needs to be, the select spans the full width of the container -- it's wider than any of the other width demos below it.

https://developer.snapappointments.com/bootstrap-select/examples/#styling

This issue was moved by caseyjhol from snapappointments/bootstrap-select-temp#53.

@ghost
Copy link
Author

ghost commented Aug 21, 2018

@caseyjhol commented on May 3, 2018, 5:31 PM UTC:

Good catch. I originally chalked this up to an issue with Bootstrap 4, but the problem is occurring with Bootstrap 3 as well.

v1.12.4: https://plnkr.co/edit/Iis43L9hSmUNdAQupYhr?p=preview
v1.13.1: https://plnkr.co/edit/XYd7d6CEyZ1cyiseLNy6?p=preview

@ghost
Copy link
Author

ghost commented Aug 21, 2018

@bazineta commented on May 6, 2018, 6:47 PM UTC:

In testing on Bootstrap 3 (lining things up to migrate to 4, and we use this great plugin a lot), the change is improved, but still not quite the same as the old version.

In one instance, we've got a select that contains only the numbers 10, 20, and 50. With the old version, 'auto' minimally sizes; it's really just wide enough to fit the numbers and the caret, which is ideal. With the new version, including this change, it's better, but still substantially wider than the old version is.

@caseyjhol
Copy link
Member

Released in v1.13.2!

@bazineta
Copy link

As mentioned previously, so far as I can see, it's still not working.

In one instance, we've got a select that contains only the numbers 10, 20, and 50. With the old version, 'auto' minimally sizes; it's really just wide enough to fit the numbers and the caret, which is ideal. With the new version, including this change, it's better, but still substantially wider than the old version is.

The width seemingly can't go below 101px, but I've not yet traced just why.

@caseyjhol
Copy link
Member

Checking into it. It appears to be working for bigger selects, at least:

v1.12.4: https://plnkr.co/edit/LvOsBZA33wu5Ry8LzgIY?p=preview
v1.13.2: https://plnkr.co/edit/fbvDUO72PVP3t8Kc717e?p=preview

@caseyjhol caseyjhol reopened this Aug 28, 2018
@bazineta
Copy link

Thanks. It's definitely working for bigger ones; it's the small ones that exhibit the problem. For this case of the 10, 20, 50 select options, the 1.12.4 release calculates the width at 58.48px, while the 1.13.2 release calculates it at 101px.

The 1.12.4 release does appear to have calculated the correct width; it's wide enough to fit the widest entry, with the button overhead accounted for, but no wider.

I'm attempting to figure out why the new release doesn't come up with the same result, but it'll be slow going with me. Will advise if I find anything.

When it arrives here:

          // Set width to whatever's larger, button title or longest option
          that.sizeInfo.selectWidth = Math.max(that.sizeInfo.totalMenuWidth, btnWidth);

btnWidth is 26, while totalMenuWidth is 101, so it's seemingly going astray fairly early.

@caseyjhol
Copy link
Member

Thanks for looking into this! I think I have an idea of what the problem might be. Hoping to get this resolved soon. The next release should be a lot sooner too.

@caseyjhol
Copy link
Member

caseyjhol commented Aug 28, 2018

@bazineta Got it figured out. You're probably going to cry when you see what the problem was. I was using some default text when creating the li elements to determine menu dimensions. The default text of "Inner text" was obviously far too long. Not sure what I was thinking.

commit 118020c

@bazineta
Copy link

I see the commit, and feel that I need a drink now...

@bazineta
Copy link

@caseyjhol Tested, and it's golden now.

@caseyjhol
Copy link
Member

Sorry I didn't catch that sooner. Appreciate all your help with it.

@caseyjhol
Copy link
Member

Released in v1.13.3!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants