-
-
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
fix #13185 - keyboard support for carousel #13787
Conversation
@@ -18,7 +18,7 @@ | |||
// ========================= | |||
|
|||
var Carousel = function (element, options) { | |||
this.$element = $(element) | |||
this.$element = $(element).on('keydown', $.proxy(this.keydown, this)) |
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.
We should probably namespace this, though the mouseenter
and mouseleave
events aren't either so I'm not so sure...
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.
mm yeah good call
lg? |
} | ||
|
||
e.preventDefault() | ||
} |
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.
This seems more fitting 😄
Carousel.prototype.keydown = function (e) {
if (!/37|39/.test(e.which)) return
this[e.which == 37 ? 'prev' : 'next']()
e.preventDefault()
}
Also, ref
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.
yeah it was a switch statement because mark and i were going back and forth with adding a first/last option
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 think the switch is a bit more elegant in this case - only one condition, etc
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.
Yeah, but the semicolons aren't that elegant.
Looking good to whom? You might wanna cc someone specific in. |
fix #13185 - keyboard support for carousel
No description provided.