-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
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
Configure affix target element #13342
Conversation
Please also include a docs update to mention this new parameter. |
this.$window = $(window) | ||
.on('scroll.bs.affix.data-api', $.proxy(this.checkPosition, this)) | ||
.on('click.bs.affix.data-api', $.proxy(this.checkPositionWithEventLoop, this)) | ||
this.$window = this.options.target ? $(this.options.target) : $(window) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think $(window)
might make more sense added to affix's defaults… rather then doing this check here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also this.$window
doesn't make much sense if the target isn't the window… should probably rename that to this.$target
/cc @fat for final review since the requested changes have been made. |
.on('scroll.bs.affix.data-api', $.proxy(this.checkPosition, this)) | ||
.on('click.bs.affix.data-api', $.proxy(this.checkPositionWithEventLoop, this)) | ||
|
||
this.$target = $(this.options.target) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you dont need this var definition to be a separate line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
look how $window
was done above for ref
Running Sauce tests on a squashed version of this PR: https://travis-ci.org/twbs/bootstrap/jobs/25362457 |
Sauce tests passed. |
No description provided.