Skip to content
New issue

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

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

Already on GitHub? # to your account

Only trim input_width to container_width when calculable. #2880

Merged
merged 1 commit into from
Sep 5, 2017

Conversation

adunkman
Copy link
Contributor

@harvesthq/chosen-developers to fix #2480.

This PR adjusts the logic used to calculate the width of the text input when in multi-select mode.

If Chosen is not visible when initialized (it’s within a hidden container), the calculated width of Chosen’s container is 0. This means that we’re setting the search input width to an initial value of -10px, which is nonsense — resulting in a fallback to it’s default width in CSS (25px).

This, of course, is adjusted after Chosen is interacted with, since it is now visible and the container width can be correctly calculated.

Since we don’t know when Chosen becomes visible, I propose we adjust the default width for the search input to be the full calculated width, and ignore the calculated container_width if it results in zero. The text doesn’t actually overflow outside of the container — the only downside is that a few pixels of space isn’t present.

Here’s a GIF of Chosen (which was rendered off-screen initially) being opened and closed. Before, the width of the search input was not contained to the container, and after it is:

screencast

If you didn’t catch it, look for the tail end of the “a” character. This is significantly better than what’s currently happening — the input is rendered at 25px wide:

Before this PR After this PR
Before Opening image image
After Opening image image


width = Math.min(container_width - 10, width)
if @container.is(':visible')
width = Math.min(@container.outerWidth() - 10, width)
Copy link
Contributor

@satchmorun satchmorun Sep 1, 2017

Choose a reason for hiding this comment

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

@adunkman and I paired on this, because I couldn't reproduce the issue in my jquery HTML test page.

While working on it, we discovered a fun issue with jquery's outerWidth method. The short of it is that if the element is hidden, one should not trust jquery to do anything sensible. For example, with a style of width: 98% (on a non-visible element), it will return 98. Le sigh.

So we needed to do a different thing in the jQuery version than in the prototype version.

Go early 2000s!

Copy link
Contributor

@braddunbar braddunbar left a comment

Choose a reason for hiding this comment

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

This is definitely better than the current behavior! 👍 💯

@adunkman adunkman merged commit c697408 into master Sep 5, 2017
@adunkman adunkman deleted the fix-cutoff-placeholder-text branch September 5, 2017 16:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default text for multiple select is being truncated
3 participants