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

[dropdown] Fix a sizer element being created every time dropdown is initialized #1496

Merged
merged 1 commit into from
Jun 3, 2020

Conversation

ryamaguchi0220
Copy link
Contributor

@ryamaguchi0220 ryamaguchi0220 commented Jun 2, 2020

Description

This PR ensures that a dropdown has only one sizer element no matter how many times it is initialized.

It looks like !module.has.sizer() of this line already ensures it, but actually does not. $sizer.length > 0 inside has.sizer() always returns false because sizer.selector is > input.sizer even though the sizer element is created as span element (I think > span.sizer is needed instead of > input.sizer).

I'm not 100% sure whether this is a bug, so feel free to close this PR if anything is wrong.

Testcase

Both test cases initialize the dropdown twice.
Try to see how many sizer elements are created via your developer tool.

Broken

Fixed

Closes

no-issue

@lubber-de lubber-de added lang/javascript Anything involving JavaScript state/awaiting-reviews Pull requests which are waiting for reviews type/bug Any issue which is a bug or PR which fixes a bug labels Jun 2, 2020
@lubber-de lubber-de added this to the 2.8.6 milestone Jun 2, 2020
Copy link
Member

@lubber-de lubber-de left a comment

Choose a reason for hiding this comment

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

LGTM 👍 , very nice catch, the selector is definately wrong.

Copy link
Contributor

@prudho prudho left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@exoego exoego left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@lubber-de lubber-de merged commit b3be317 into fomantic:develop Jun 3, 2020
@lubber-de lubber-de added tag/next-release/nightly Any issue which has a corresponding PR which has been merged and is available in the nightly build and removed state/awaiting-reviews Pull requests which are waiting for reviews labels Jun 3, 2020
@ryamaguchi0220 ryamaguchi0220 deleted the develop branch June 18, 2020 06:53
@lubber-de lubber-de removed the tag/next-release/nightly Any issue which has a corresponding PR which has been merged and is available in the nightly build label Aug 25, 2020
@lubber-de
Copy link
Member

@all-contributors please add @ryamaguchi0220 for code

@allcontributors
Copy link
Contributor

@lubber-de

@ryamaguchi0220 already contributed before to code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/javascript Anything involving JavaScript type/bug Any issue which is a bug or PR which fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants