Skip to content

Commit

Permalink
Fix carousel "hover" behavior on touch-enabled devices
Browse files Browse the repository at this point in the history
* Add carousel mouse listeners even if touch events enabled

- touch events are enabled not just on "mobile", just also on
touch-enabled desktop/laptop devices; additionally, it's possible to
pair a mouse with traditionally touch-only devices (e.g. Android
phones/tablets); currently, in these situations the carousel WON'T pause
even when using a mouse

* Restart cycle after touchend

as `mouseenter` is fired as part of the touch compatibility events, the
previous change results in carousels which cycle until the user
tapped/interacted with them. after that they stop cycling (as
`mouseleave` is not sent to the carousel after user scrolled/tapped
away).
this fix resets the cycling after `touchend` - essentially returning
to the previous behavior, where on touch the carousel essentially never
pauses, but now with the previous fix it at least pauses correctly for
mouse users on touch-enabled devices.
includes documentation for this new behavior.
  • Loading branch information
patrickhlauke authored Apr 17, 2017
1 parent 7ffb61a commit 6ae5fb1
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 35 deletions.
3 changes: 2 additions & 1 deletion docs/components/carousel.md
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ Options can be passed via data attributes or JavaScript. For data attributes, ap
<td>pause</td>
<td>string | boolean</td>
<td>"hover"</td>
<td>If set to <code>"hover"</code>, pauses the cycling of the carousel on <code>mouseenter</code> and resumes the cycling of the carousel on <code>mouseleave</code>. If set to <code>false</code>, hovering over the carousel won't pause it.</td>
<td><p>If set to <code>"hover"</code>, pauses the cycling of the carousel on <code>mouseenter</code> and resumes the cycling of the carousel on <code>mouseleave</code>. If set to <code>false</code>, hovering over the carousel won't pause it.</p>
<p>On touch-enabled devices, when set to <code>"hover"</code>, cycling will pause on <code>touchend</code> (once the user finished interacting with the carousel) for two intervals, before automatically resuming. Note that this is in addition to the above mouse behavior.</p></td>
</tr>
<tr>
<td>ride</td>
Expand Down
41 changes: 30 additions & 11 deletions js/src/carousel.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,16 @@ const Carousel = (($) => {
* ------------------------------------------------------------------------
*/

const NAME = 'carousel'
const VERSION = '4.0.0-alpha.6'
const DATA_KEY = 'bs.carousel'
const EVENT_KEY = `.${DATA_KEY}`
const DATA_API_KEY = '.data-api'
const JQUERY_NO_CONFLICT = $.fn[NAME]
const TRANSITION_DURATION = 600
const ARROW_LEFT_KEYCODE = 37 // KeyboardEvent.which value for left arrow key
const ARROW_RIGHT_KEYCODE = 39 // KeyboardEvent.which value for right arrow key
const NAME = 'carousel'
const VERSION = '4.0.0-alpha.6'
const DATA_KEY = 'bs.carousel'
const EVENT_KEY = `.${DATA_KEY}`
const DATA_API_KEY = '.data-api'
const JQUERY_NO_CONFLICT = $.fn[NAME]
const TRANSITION_DURATION = 600
const ARROW_LEFT_KEYCODE = 37 // KeyboardEvent.which value for left arrow key
const ARROW_RIGHT_KEYCODE = 39 // KeyboardEvent.which value for right arrow key
const TOUCHEVENT_COMPAT_WAIT = 500 // Time for mouse compat events to fire after touch

const Default = {
interval : 5000,
Expand Down Expand Up @@ -56,6 +57,7 @@ const Carousel = (($) => {
KEYDOWN : `keydown${EVENT_KEY}`,
MOUSEENTER : `mouseenter${EVENT_KEY}`,
MOUSELEAVE : `mouseleave${EVENT_KEY}`,
TOUCHEND : `touchend${EVENT_KEY}`,
LOAD_DATA_API : `load${EVENT_KEY}${DATA_API_KEY}`,
CLICK_DATA_API : `click${EVENT_KEY}${DATA_API_KEY}`
}
Expand Down Expand Up @@ -98,6 +100,8 @@ const Carousel = (($) => {
this._isPaused = false
this._isSliding = false

this.touchTimeout = null

this._config = this._getConfig(config)
this._element = $(element)[0]
this._indicatorsElement = $(this._element).find(Selector.INDICATORS)[0]
Expand Down Expand Up @@ -227,11 +231,26 @@ const Carousel = (($) => {
.on(Event.KEYDOWN, (event) => this._keydown(event))
}

if (this._config.pause === 'hover' &&
!('ontouchstart' in document.documentElement)) {
if (this._config.pause === 'hover') {
$(this._element)
.on(Event.MOUSEENTER, (event) => this.pause(event))
.on(Event.MOUSELEAVE, (event) => this.cycle(event))
if ('ontouchstart' in document.documentElement) {
// if it's a touch-enabled device, mouseenter/leave are fired as
// part of the mouse compatibility events on first tap - the carousel
// would stop cycling until user tapped out of it;
// here, we listen for touchend, explicitly pause the carousel
// (as if it's the second time we tap on it, mouseenter compat event
// is NOT fired) and after a timeout (to allow for mouse compatibility
// events to fire) we explicitly restart cycling
$(this._element).on(Event.TOUCHEND, () => {
this.pause()
if (this.touchTimeout) {
clearTimeout(this.touchTimeout)
}
this.touchTimeout = setTimeout((event) => this.cycle(event), TOUCHEVENT_COMPAT_WAIT + this._config.interval)
})
}
}
}

Expand Down
23 changes: 0 additions & 23 deletions js/tests/unit/carousel.js
Original file line number Diff line number Diff line change
Expand Up @@ -654,29 +654,6 @@ $(function () {
assert.strictEqual($template.find('.carousel-item')[0], $template.find('.active')[0], 'first item still active after left arrow press in <textarea>')
})

QUnit.test('should only add mouseenter and mouseleave listeners when not on mobile', function (assert) {
assert.expect(2)
var isMobile = 'ontouchstart' in document.documentElement
var templateHTML = '<div id="myCarousel" class="carousel" data-interval="false" data-pause="hover">'
+ '<div class="carousel-inner">'
+ '<div id="first" class="carousel-item active">'
+ '<img alt="">'
+ '</div>'
+ '<div id="second" class="carousel-item">'
+ '<img alt="">'
+ '</div>'
+ '<div id="third" class="carousel-item">'
+ '<img alt="">'
+ '</div>'
+ '</div>'
+ '</div>'
var $template = $(templateHTML).bootstrapCarousel()

$.each(['mouseover', 'mouseout'], function (i, type) {
assert.strictEqual(type in $._data($template[0], 'events'), !isMobile, 'does' + (isMobile ? ' not' : '') + ' listen for ' + type + ' events')
})
})

QUnit.test('should wrap around from end to start when wrap option is true', function (assert) {
assert.expect(3)
var carouselHTML = '<div id="carousel-example-generic" class="carousel slide" data-wrap="true">'
Expand Down

0 comments on commit 6ae5fb1

Please sign in to comment.