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

Use undefined object and jQuery trigger method #1356

Closed
wants to merge 1 commit into from
Closed

Use undefined object and jQuery trigger method #1356

wants to merge 1 commit into from

Conversation

vsn4ik
Copy link
Collaborator

@vsn4ik vsn4ik commented Apr 8, 2016

  1. Use native focus method if possible
  2. http://jsperf.com/is-undefined
  3. Use $.fn.trigger for unification. Deprecate event shorthand methods (jQuery v4, Deprecate event shorthand methods jquery/jquery#3214)

isOptgroup = this.parentNode.tagName === 'OPTGROUP',
isDisabled = this.disabled || (isOptgroup && this.parentNode.disabled);

if (icon !== '' && isDisabled) {
icon = '<span>' + icon + '</span>';
}

if (that.options.hideDisabled && (isDisabled && !isOptgroup || this.parentNode.disabled && isOptgroup)) {
if (that.options.hideDisabled && isDisabled) {
Copy link
Member

Choose a reason for hiding this comment

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

This line needs to be that.options.hideDisabled && (isDisabled && !isOptgroup || this.parentNode.disabled && isOptgroup). With just that.options.hideDisabled && isDisabled, if an optgroup has a disabled option in it, the optgroup's header is hidden. I guess we could make a variable called something like isOptgroupDisabled and set it equal to this.parentNode.disabled && isOptgroup, and then set the if statement to if (that.options.hideDisabled && (isDisabled && !isOptgroup || isOptgroupDisabled)).

@vsn4ik
Copy link
Collaborator Author

vsn4ik commented Apr 8, 2016

@caseyjhol better looks now?

liIndex--;
return;
}

Copy link
Member

Choose a reason for hiding this comment

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

This needs to remain, otherwise disabled options within an optgroup stay visible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

caseyjhol added a commit that referenced this pull request May 13, 2016
vsn4ik referenced this pull request Sep 13, 2016
@vsn4ik vsn4ik closed this Jan 19, 2017
@vsn4ik vsn4ik deleted the use-undefined_object branch January 19, 2017 21:20
avantika-gupta-jtg pushed a commit to JoshLabs/bootstrap-select that referenced this pull request May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants