From 286f5e6614257e45f881545931caeb329c9c999a Mon Sep 17 00:00:00 2001 From: Johann-S Date: Sat, 3 Mar 2018 23:04:11 +0200 Subject: [PATCH 1/9] Add touch support in our carousel with HammerJS. --- README.md | 2 +- _config.yml | 2 + build/build-plugins.js | 11 +- build/generate-sri.js | 4 + build/rollup.config.js | 26 ++-- js/src/carousel.js | 54 +++++--- js/tests/index.html | 4 + js/tests/karma.conf.js | 17 ++- js/tests/unit/.eslintrc.json | 4 +- js/tests/unit/carousel.js | 127 ++++++++++++++++++ js/tests/visual/carousel.html | 1 + package-lock.json | 39 ++++++ package.json | 8 +- site/_includes/scripts.html | 1 + site/docs/4.1/components/carousel.md | 8 ++ site/docs/4.1/getting-started/contents.md | 2 +- site/docs/4.1/getting-started/introduction.md | 2 +- 17 files changed, 269 insertions(+), 43 deletions(-) diff --git a/README.md b/README.md index 8694b686f14e..17680e642dec 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,7 @@ bootstrap/ └── bootstrap.min.js.map ``` -We provide compiled CSS and JS (`bootstrap.*`), as well as compiled and minified CSS and JS (`bootstrap.min.*`). [source maps](https://developers.google.com/web/tools/chrome-devtools/debug/readability/source-maps) (`bootstrap.*.map`) are available for use with certain browsers' developer tools. Bundled JS files (`bootstrap.bundle.js` and minified `bootstrap.bundle.min.js`) include [Popper](https://popper.js.org/), but not [jQuery](https://jquery.com/). +We provide compiled CSS and JS (`bootstrap.*`), as well as compiled and minified CSS and JS (`bootstrap.min.*`). [source maps](https://developers.google.com/web/tools/chrome-devtools/debug/readability/source-maps) (`bootstrap.*.map`) are available for use with certain browsers' developer tools. Bundled JS files (`bootstrap.bundle.js` and minified `bootstrap.bundle.min.js`) include [Popper](https://popper.js.org/) and [HammerJS](https://hammerjs.github.io/), but not [jQuery](https://jquery.com/). ## Bugs and feature requests diff --git a/_config.yml b/_config.yml index e073dce9b9df..7fab354119bb 100644 --- a/_config.yml +++ b/_config.yml @@ -58,6 +58,8 @@ cdn: jquery_hash: "sha384-q8i/X+965DzO0rT7abK41JStQIAqVgRVzpbzo5smXKp4YfRvH+8abtTE1Pi6jizo" popper: "https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.14.4/umd/popper.min.js" popper_hash: "sha384-GM0Y80ecpwKxF1D5XCrGanKusGDy9WW0O2sSM84neB4iFhvKp3fwnoIRnPsQcN1R" + hammer: "https://cdnjs.cloudflare.com/ajax/libs/hammer.js/2.0.8/hammer.min.js" + hammer_hash: "sha384-Cs3dgUx6+jDxxuqHvVH8Onpyj2LF1gKZurLDlhqzuJmUqVYMJ0THTWpxK5Z086Zm" toc: min_level: 2 diff --git a/build/build-plugins.js b/build/build-plugins.js index 1de65b426dc0..299f502d9d94 100644 --- a/build/build-plugins.js +++ b/build/build-plugins.js @@ -5,10 +5,10 @@ * Licensed under MIT (https://github.com/twbs/bootstrap/blob/master/LICENSE) */ -const path = require('path') -const rollup = require('rollup') -const babel = require('rollup-plugin-babel') -const banner = require('./banner.js') +const path = require('path') +const rollup = require('rollup') +const babel = require('rollup-plugin-babel') +const banner = require('./banner.js') const TEST = process.env.NODE_ENV === 'test' const plugins = [ @@ -41,8 +41,9 @@ const rootPath = TEST ? '../js/coverage/dist/' : '../js/dist/' function build(plugin) { console.log(`Building ${plugin} plugin...`) - const external = ['jquery', 'popper.js'] + const external = ['hammerjs', 'jquery', 'popper.js'] const globals = { + hammerjs: 'Hammer', jquery: 'jQuery', // Ensure we use jQuery which is always available even in noConflict mode 'popper.js': 'Popper' } diff --git a/build/generate-sri.js b/build/generate-sri.js index 6929097703ec..13b90db1ceb2 100644 --- a/build/generate-sri.js +++ b/build/generate-sri.js @@ -42,6 +42,10 @@ const files = [ { file: 'node_modules/popper.js/dist/umd/popper.min.js', configPropertyName: 'popper_hash' + }, + { + file: 'node_modules/hammerjs/hammer.min.js', + configPropertyName: 'hammer_hash' } ] diff --git a/build/rollup.config.js b/build/rollup.config.js index c8acf7a9e92b..72e3951fa625 100644 --- a/build/rollup.config.js +++ b/build/rollup.config.js @@ -1,12 +1,13 @@ -const path = require('path') -const babel = require('rollup-plugin-babel') -const resolve = require('rollup-plugin-node-resolve') -const banner = require('./banner.js') +const path = require('path') +const babel = require('rollup-plugin-babel') +const resolve = require('rollup-plugin-node-resolve') +const commonjs = require('rollup-plugin-commonjs') +const banner = require('./banner.js') const BUNDLE = process.env.BUNDLE === 'true' -let fileDest = 'bootstrap.js' -const external = ['jquery', 'popper.js'] +let fileDest = 'bootstrap.js' +const external = ['jquery', 'hammerjs', 'popper.js'] const plugins = [ babel({ exclude: 'node_modules/**', // Only transpile our source code @@ -21,15 +22,22 @@ const plugins = [ ] const globals = { jquery: 'jQuery', // Ensure we use jQuery which is always available even in noConflict mode + hammerjs: 'Hammer', 'popper.js': 'Popper' } if (BUNDLE) { fileDest = 'bootstrap.bundle.js' - // Remove last entry in external array to bundle Popper - external.pop() + // We just keep jQuery as external + external.length = 1 delete globals['popper.js'] - plugins.push(resolve()) + delete globals.hammerjs + plugins.push( + commonjs({ + include: 'node_modules/**' + }), + resolve() + ) } module.exports = { diff --git a/js/src/carousel.js b/js/src/carousel.js index fcc78af6f104..b2765ac5a8be 100644 --- a/js/src/carousel.js +++ b/js/src/carousel.js @@ -1,4 +1,5 @@ import $ from 'jquery' +import Hammer from 'hammerjs' import Util from './util' /** @@ -23,13 +24,15 @@ const JQUERY_NO_CONFLICT = $.fn[NAME] 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 HAMMER_ENABLED = typeof Hammer !== 'undefined' const Default = { interval : 5000, keyboard : true, slide : false, pause : 'hover', - wrap : true + wrap : true, + touch : true } const DefaultType = { @@ -37,7 +40,8 @@ const DefaultType = { keyboard : 'boolean', slide : '(boolean|string)', pause : '(string|boolean)', - wrap : 'boolean' + wrap : 'boolean', + touch : 'boolean' } const Direction = { @@ -55,7 +59,9 @@ const Event = { 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}` + CLICK_DATA_API : `click${EVENT_KEY}${DATA_API_KEY}`, + SWIPELEFT : 'swipeleft', + SWIPERIGHT : 'swiperight' } const ClassName = { @@ -84,21 +90,30 @@ const Selector = { * Class Definition * ------------------------------------------------------------------------ */ - class Carousel { constructor(element, config) { - this._items = null - this._interval = null - this._activeElement = null - - this._isPaused = false - this._isSliding = false - - this.touchTimeout = null - - this._config = this._getConfig(config) - this._element = $(element)[0] - this._indicatorsElement = this._element.querySelector(Selector.INDICATORS) + this._items = null + this._interval = null + this._activeElement = null + this._isPaused = false + this._isSliding = false + this.touchTimeout = null + this.hammer = null + + this._config = this._getConfig(config) + this._element = element + this._indicatorsElement = this._element.querySelector(Selector.INDICATORS) + this._touchSupported = 'ontouchstart' in document.documentElement || navigator.maxTouchPoints > 0 + + if (HAMMER_ENABLED && this._touchSupported && this._config.touch) { + this.hammer = new Hammer(this._element, { + recognizers: [[ + Hammer.Swipe, { + direction: Hammer.DIRECTION_HORIZONTAL + } + ]] + }) + } this._addEventListeners() } @@ -226,11 +241,16 @@ class Carousel { .on(Event.KEYDOWN, (event) => this._keydown(event)) } + if (this.hammer) { + this.hammer.on(Event.SWIPELEFT, () => this.next()) + this.hammer.on(Event.SWIPERIGHT, () => this.prev()) + } + 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 (this._touchSupported) { // 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; diff --git a/js/tests/index.html b/js/tests/index.html index ce4a0e3081a3..201e15f2a81a 100644 --- a/js/tests/index.html +++ b/js/tests/index.html @@ -20,6 +20,7 @@ }()) + @@ -28,6 +29,9 @@ + + + + + {%- if site.github -%} diff --git a/site/docs/4.1/components/carousel.md b/site/docs/4.1/components/carousel.md index 543b06430a6a..6bfb352069fb 100644 --- a/site/docs/4.1/components/carousel.md +++ b/site/docs/4.1/components/carousel.md @@ -12,6 +12,8 @@ The carousel is a slideshow for cycling through a series of content, built with In browsers where the [Page Visibility API](https://www.w3.org/TR/page-visibility/) is supported, the carousel will avoid sliding when the webpage is not visible to the user (such as when the browser tab is inactive, the browser window is minimized, etc.). +The carousel supports swipe gestures (left and right) using [HammerJS]({{ site.cdn.hammer }}). For this to function correctly you need to include HammerJS before Bootstrap or use `bootstrap.bundle.min.js` / `bootstrap.bundle.js` which contains HammerJS. + Please be aware that nested carousels are not supported, and carousels are generally not compliant with accessibility standards. Lastly, if you're building our JavaScript from source, it [requires `util.js`]({{ site.baseurl }}/docs/{{ site.docs_version }}/getting-started/javascript/#util). @@ -281,6 +283,12 @@ Options can be passed via data attributes or JavaScript. For data attributes, ap true Whether the carousel should cycle continuously or have hard stops. + + touch + boolean + true + Whether the carousel should handle touch event and allow swipe left/right. + diff --git a/site/docs/4.1/getting-started/contents.md b/site/docs/4.1/getting-started/contents.md index fd38e2fba5e9..0c210d0438aa 100644 --- a/site/docs/4.1/getting-started/contents.md +++ b/site/docs/4.1/getting-started/contents.md @@ -38,7 +38,7 @@ bootstrap/ └── bootstrap.min.js.map {% endhighlight %} -This is the most basic form of Bootstrap: precompiled files for quick drop-in usage in nearly any web project. We provide compiled CSS and JS (`bootstrap.*`), as well as compiled and minified CSS and JS (`bootstrap.min.*`). [source maps](https://developers.google.com/web/tools/chrome-devtools/javascript/source-maps) (`bootstrap.*.map`) are available for use with certain browsers' developer tools. Bundled JS files (`bootstrap.bundle.js` and minified `bootstrap.bundle.min.js`) include [Popper](https://popper.js.org/), but not [jQuery](https://jquery.com/). +This is the most basic form of Bootstrap: precompiled files for quick drop-in usage in nearly any web project. We provide compiled CSS and JS (`bootstrap.*`), as well as compiled and minified CSS and JS (`bootstrap.min.*`). [source maps](https://developers.google.com/web/tools/chrome-devtools/javascript/source-maps) (`bootstrap.*.map`) are available for use with certain browsers' developer tools. Bundled JS files (`bootstrap.bundle.js` and minified `bootstrap.bundle.min.js`) include [Popper](https://popper.js.org/) and [HammerJS](https://hammerjs.github.io/), but not [jQuery](https://jquery.com/). ## CSS files diff --git a/site/docs/4.1/getting-started/introduction.md b/site/docs/4.1/getting-started/introduction.md index 55ced2f9c318..0ad797301050 100644 --- a/site/docs/4.1/getting-started/introduction.md +++ b/site/docs/4.1/getting-started/introduction.md @@ -37,7 +37,7 @@ We use [jQuery's slim build](https://blog.jquery.com/2016/06/09/jquery-3-0-final Curious which components explicitly require jQuery, our JS, and Popper.js? Click the show components link below. If you're at all unsure about the general page structure, keep reading for an example page template. -Our `bootstrap.bundle.js` and `bootstrap.bundle.min.js` include [Popper](https://popper.js.org/), but not [jQuery](https://jquery.com/). For more information about what's included in Bootstrap, please see our [contents]({{ site.baseurl }}/docs/{{ site.docs_version }}/getting-started/contents/#precompiled-bootstrap) section. +Our `bootstrap.bundle.js` and `bootstrap.bundle.min.js` include [Popper](https://popper.js.org/) and [HammerJS](https://hammerjs.github.io/), but not [jQuery](https://jquery.com/). For more information about what's included in Bootstrap, please see our [contents]({{ site.baseurl }}/docs/{{ site.docs_version }}/getting-started/contents/#precompiled-bootstrap) section.
Show components requiring JavaScript From cb6450e84fe09189779cb305432680b701b3c6a5 Mon Sep 17 00:00:00 2001 From: Johann-S Date: Sun, 14 Oct 2018 13:59:51 +0200 Subject: [PATCH 2/9] swipe left/right without hammerjs --- README.md | 2 +- _config.yml | 2 - build/build-plugins.js | 11 +- build/generate-sri.js | 4 - build/rollup.config.js | 26 ++--- js/src/carousel.js | 100 ++++++++++++------ js/tests/index.html | 1 - js/tests/karma.conf.js | 11 +- js/tests/unit/carousel.js | 73 ++++++------- js/tests/visual/carousel.html | 1 - package-lock.json | 33 ------ package.json | 7 +- site/_includes/scripts.html | 1 - site/docs/4.1/components/carousel.md | 4 +- site/docs/4.1/getting-started/contents.md | 2 +- site/docs/4.1/getting-started/introduction.md | 2 +- 16 files changed, 122 insertions(+), 158 deletions(-) diff --git a/README.md b/README.md index 17680e642dec..8694b686f14e 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,7 @@ bootstrap/ └── bootstrap.min.js.map ``` -We provide compiled CSS and JS (`bootstrap.*`), as well as compiled and minified CSS and JS (`bootstrap.min.*`). [source maps](https://developers.google.com/web/tools/chrome-devtools/debug/readability/source-maps) (`bootstrap.*.map`) are available for use with certain browsers' developer tools. Bundled JS files (`bootstrap.bundle.js` and minified `bootstrap.bundle.min.js`) include [Popper](https://popper.js.org/) and [HammerJS](https://hammerjs.github.io/), but not [jQuery](https://jquery.com/). +We provide compiled CSS and JS (`bootstrap.*`), as well as compiled and minified CSS and JS (`bootstrap.min.*`). [source maps](https://developers.google.com/web/tools/chrome-devtools/debug/readability/source-maps) (`bootstrap.*.map`) are available for use with certain browsers' developer tools. Bundled JS files (`bootstrap.bundle.js` and minified `bootstrap.bundle.min.js`) include [Popper](https://popper.js.org/), but not [jQuery](https://jquery.com/). ## Bugs and feature requests diff --git a/_config.yml b/_config.yml index 7fab354119bb..e073dce9b9df 100644 --- a/_config.yml +++ b/_config.yml @@ -58,8 +58,6 @@ cdn: jquery_hash: "sha384-q8i/X+965DzO0rT7abK41JStQIAqVgRVzpbzo5smXKp4YfRvH+8abtTE1Pi6jizo" popper: "https://cdnjs.cloudflare.com/ajax/libs/popper.js/1.14.4/umd/popper.min.js" popper_hash: "sha384-GM0Y80ecpwKxF1D5XCrGanKusGDy9WW0O2sSM84neB4iFhvKp3fwnoIRnPsQcN1R" - hammer: "https://cdnjs.cloudflare.com/ajax/libs/hammer.js/2.0.8/hammer.min.js" - hammer_hash: "sha384-Cs3dgUx6+jDxxuqHvVH8Onpyj2LF1gKZurLDlhqzuJmUqVYMJ0THTWpxK5Z086Zm" toc: min_level: 2 diff --git a/build/build-plugins.js b/build/build-plugins.js index 299f502d9d94..1de65b426dc0 100644 --- a/build/build-plugins.js +++ b/build/build-plugins.js @@ -5,10 +5,10 @@ * Licensed under MIT (https://github.com/twbs/bootstrap/blob/master/LICENSE) */ -const path = require('path') -const rollup = require('rollup') -const babel = require('rollup-plugin-babel') -const banner = require('./banner.js') +const path = require('path') +const rollup = require('rollup') +const babel = require('rollup-plugin-babel') +const banner = require('./banner.js') const TEST = process.env.NODE_ENV === 'test' const plugins = [ @@ -41,9 +41,8 @@ const rootPath = TEST ? '../js/coverage/dist/' : '../js/dist/' function build(plugin) { console.log(`Building ${plugin} plugin...`) - const external = ['hammerjs', 'jquery', 'popper.js'] + const external = ['jquery', 'popper.js'] const globals = { - hammerjs: 'Hammer', jquery: 'jQuery', // Ensure we use jQuery which is always available even in noConflict mode 'popper.js': 'Popper' } diff --git a/build/generate-sri.js b/build/generate-sri.js index 13b90db1ceb2..6929097703ec 100644 --- a/build/generate-sri.js +++ b/build/generate-sri.js @@ -42,10 +42,6 @@ const files = [ { file: 'node_modules/popper.js/dist/umd/popper.min.js', configPropertyName: 'popper_hash' - }, - { - file: 'node_modules/hammerjs/hammer.min.js', - configPropertyName: 'hammer_hash' } ] diff --git a/build/rollup.config.js b/build/rollup.config.js index 72e3951fa625..c8acf7a9e92b 100644 --- a/build/rollup.config.js +++ b/build/rollup.config.js @@ -1,13 +1,12 @@ -const path = require('path') -const babel = require('rollup-plugin-babel') -const resolve = require('rollup-plugin-node-resolve') -const commonjs = require('rollup-plugin-commonjs') -const banner = require('./banner.js') +const path = require('path') +const babel = require('rollup-plugin-babel') +const resolve = require('rollup-plugin-node-resolve') +const banner = require('./banner.js') const BUNDLE = process.env.BUNDLE === 'true' -let fileDest = 'bootstrap.js' -const external = ['jquery', 'hammerjs', 'popper.js'] +let fileDest = 'bootstrap.js' +const external = ['jquery', 'popper.js'] const plugins = [ babel({ exclude: 'node_modules/**', // Only transpile our source code @@ -22,22 +21,15 @@ const plugins = [ ] const globals = { jquery: 'jQuery', // Ensure we use jQuery which is always available even in noConflict mode - hammerjs: 'Hammer', 'popper.js': 'Popper' } if (BUNDLE) { fileDest = 'bootstrap.bundle.js' - // We just keep jQuery as external - external.length = 1 + // Remove last entry in external array to bundle Popper + external.pop() delete globals['popper.js'] - delete globals.hammerjs - plugins.push( - commonjs({ - include: 'node_modules/**' - }), - resolve() - ) + plugins.push(resolve()) } module.exports = { diff --git a/js/src/carousel.js b/js/src/carousel.js index b2765ac5a8be..5b6209460a4e 100644 --- a/js/src/carousel.js +++ b/js/src/carousel.js @@ -1,7 +1,3 @@ -import $ from 'jquery' -import Hammer from 'hammerjs' -import Util from './util' - /** * -------------------------------------------------------------------------- * Bootstrap (v4.1.3): carousel.js @@ -9,6 +5,9 @@ import Util from './util' * -------------------------------------------------------------------------- */ +import $ from 'jquery' +import Util from './util' + /** * ------------------------------------------------------------------------ * Constants @@ -24,7 +23,7 @@ const JQUERY_NO_CONFLICT = $.fn[NAME] 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 HAMMER_ENABLED = typeof Hammer !== 'undefined' +const SWIPE_THRESHOLD = 40 const Default = { interval : 5000, @@ -58,10 +57,10 @@ const Event = { MOUSEENTER : `mouseenter${EVENT_KEY}`, MOUSELEAVE : `mouseleave${EVENT_KEY}`, TOUCHEND : `touchend${EVENT_KEY}`, + TOUCHSTART : `touchstart${EVENT_KEY}`, + TOUCHMOVE : `touchmove${EVENT_KEY}`, LOAD_DATA_API : `load${EVENT_KEY}${DATA_API_KEY}`, - CLICK_DATA_API : `click${EVENT_KEY}${DATA_API_KEY}`, - SWIPELEFT : 'swipeleft', - SWIPERIGHT : 'swiperight' + CLICK_DATA_API : `click${EVENT_KEY}${DATA_API_KEY}` } const ClassName = { @@ -98,22 +97,13 @@ class Carousel { this._isPaused = false this._isSliding = false this.touchTimeout = null - this.hammer = null + this.touchStartX = 0 + this.touchDeltaX = 0 this._config = this._getConfig(config) this._element = element this._indicatorsElement = this._element.querySelector(Selector.INDICATORS) - this._touchSupported = 'ontouchstart' in document.documentElement || navigator.maxTouchPoints > 0 - - if (HAMMER_ENABLED && this._touchSupported && this._config.touch) { - this.hammer = new Hammer(this._element, { - recognizers: [[ - Hammer.Swipe, { - direction: Hammer.DIRECTION_HORIZONTAL - } - ]] - }) - } + this._touchSupported = 'ontouchstart' in document.documentElement this._addEventListeners() } @@ -235,22 +225,65 @@ class Carousel { return config } + _handleSwipe() { + const absDeltax = Math.abs(this.touchDeltaX) + + if (absDeltax <= SWIPE_THRESHOLD) { + return + } + + const direction = absDeltax / this.touchDeltaX + + // swipe left + if (direction > 0) { + this.prev() + } + + // swipe right + if (direction < 0) { + this.next() + } + } + _addEventListeners() { if (this._config.keyboard) { $(this._element) .on(Event.KEYDOWN, (event) => this._keydown(event)) } - if (this.hammer) { - this.hammer.on(Event.SWIPELEFT, () => this.next()) - this.hammer.on(Event.SWIPERIGHT, () => this.prev()) - } - if (this._config.pause === 'hover') { $(this._element) .on(Event.MOUSEENTER, (event) => this.pause(event)) .on(Event.MOUSELEAVE, (event) => this.cycle(event)) - if (this._touchSupported) { + } + + this._addTouchEventListeners() + } + + _addTouchEventListeners() { + if (!this._touchSupported) { + return + } + + $(this._element).on(Event.TOUCHSTART, (event) => { + this.touchStartX = event.originalEvent.touches[0].pageX + }) + + $(this._element).on(Event.TOUCHMOVE, (event) => { + event.preventDefault() + + // ensure swiping with one touch and not pinching + if (event.originalEvent.touches.length > 1) { + return + } + + this.touchDeltaX = event.originalEvent.touches[0].pageX - this.touchStartX + }) + + $(this._element).on(Event.TOUCHEND, () => { + this._handleSwipe() + + if (this._config.pause === 'hover') { // 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; @@ -258,15 +291,14 @@ class 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) - }) + + this.pause() + if (this.touchTimeout) { + clearTimeout(this.touchTimeout) + } + this.touchTimeout = setTimeout((event) => this.cycle(event), TOUCHEVENT_COMPAT_WAIT + this._config.interval) } - } + }) } _keydown(event) { diff --git a/js/tests/index.html b/js/tests/index.html index 201e15f2a81a..1bcdc5380e2e 100644 --- a/js/tests/index.html +++ b/js/tests/index.html @@ -20,7 +20,6 @@ }()) - diff --git a/js/tests/karma.conf.js b/js/tests/karma.conf.js index c3c64e8850d3..807f977d96da 100644 --- a/js/tests/karma.conf.js +++ b/js/tests/karma.conf.js @@ -12,16 +12,16 @@ const jqueryFile = process.env.USE_OLD_JQUERY ? 'https://code.jquery.com/jquery- const bundle = process.env.BUNDLE === 'true' const browserStack = process.env.BROWSER === 'true' -const plugins = [ - 'karma-qunit', - 'karma-sinon' -] - const frameworks = [ 'qunit', 'sinon' ] +const plugins = [ + 'karma-qunit', + 'karma-sinon' +] + const reporters = ['dots'] const detectBrowsers = { @@ -48,7 +48,6 @@ const customLaunchers = { let files = [ 'node_modules/popper.js/dist/umd/popper.min.js', - 'node_modules/hammerjs/hammer.min.js', 'node_modules/hammer-simulator/index.js' ] diff --git a/js/tests/unit/carousel.js b/js/tests/unit/carousel.js index 0bc52219e4ed..e416ab20e427 100644 --- a/js/tests/unit/carousel.js +++ b/js/tests/unit/carousel.js @@ -3,8 +3,6 @@ $(function () { window.Carousel = typeof bootstrap !== 'undefined' ? bootstrap.Carousel : Carousel - var touchSupported = 'ontouchstart' in document.documentElement || navigator.maxTouchPoints > 0 - QUnit.module('carousel plugin') QUnit.test('should be defined on jQuery object', function (assert) { @@ -1009,13 +1007,8 @@ $(function () { }) QUnit.test('should allow swiperight and call prev', function (assert) { - if (!touchSupported) { - assert.expect(0) - - return - } - - assert.expect(2) + Simulator.setType('touch') + assert.expect(3) var done = assert.async() document.documentElement.ontouchstart = $.noop @@ -1035,10 +1028,13 @@ $(function () { $carousel.appendTo('#qunit-fixture') var $item = $('#item') $carousel.bootstrapCarousel() + var carousel = $carousel.data('bs.carousel') + var spy = sinon.spy(carousel, 'prev') $carousel.one('slid.bs.carousel', function () { assert.ok(true, 'slid event fired') assert.ok($item.hasClass('active')) + assert.ok(spy.called) delete document.documentElement.ontouchstart done() }) @@ -1049,40 +1045,10 @@ $(function () { }) }) - QUnit.test('should not use HammerJS when touch option is false', function (assert) { - assert.expect(1) - - var $carousel = $('
').appendTo('#qunit-fixture') - $carousel.bootstrapCarousel({ - touch: false - }) - - var carousel = $carousel.data('bs.carousel') - - assert.strictEqual(carousel.hammer, null) - }) - - QUnit.test('should use HammerJS when touch option is true', function (assert) { - assert.expect(1) - - document.documentElement.ontouchstart = $.noop - - var $carousel = $('
').appendTo('#qunit-fixture') - $carousel.bootstrapCarousel() - - var carousel = $carousel.data('bs.carousel') - - assert.ok(carousel.hammer !== null) - }) - QUnit.test('should allow swipeleft and call next', function (assert) { - if (!touchSupported) { - assert.expect(0) - - return - } + assert.expect(3) + Simulator.setType('touch') - assert.expect(2) var done = assert.async() document.documentElement.ontouchstart = $.noop @@ -1102,11 +1068,13 @@ $(function () { $carousel.appendTo('#qunit-fixture') var $item = $('#item') $carousel.bootstrapCarousel() + var carousel = $carousel.data('bs.carousel') + var spy = sinon.spy(carousel, 'next') $carousel.one('slid.bs.carousel', function () { assert.ok(true, 'slid event fired') assert.ok(!$item.hasClass('active')) - delete document.documentElement.ontouchstart + assert.ok(spy.called) done() }) @@ -1116,4 +1084,25 @@ $(function () { deltaY: 0 }) }) + + QUnit.test('should not allow pinch', function (assert) { + assert.expect(0) + Simulator.setType('touch') + var done = assert.async() + document.documentElement.ontouchstart = $.noop + + var carouselHTML = '
' + var $carousel = $(carouselHTML) + $carousel.appendTo('#qunit-fixture') + $carousel.bootstrapCarousel() + + Simulator.gestures.swipe($carousel[0], { + pos: [300, 10], + deltaX: -300, + deltaY: 0, + touches: 2 + }, function () { + done() + }) + }) }) diff --git a/js/tests/visual/carousel.html b/js/tests/visual/carousel.html index cd917caa627e..630f870cf4e0 100644 --- a/js/tests/visual/carousel.html +++ b/js/tests/visual/carousel.html @@ -46,7 +46,6 @@

Carousel Bootstrap Visual Test

- - {%- if site.github -%} diff --git a/site/docs/4.1/components/carousel.md b/site/docs/4.1/components/carousel.md index 6bfb352069fb..b568a02cd585 100644 --- a/site/docs/4.1/components/carousel.md +++ b/site/docs/4.1/components/carousel.md @@ -12,8 +12,6 @@ The carousel is a slideshow for cycling through a series of content, built with In browsers where the [Page Visibility API](https://www.w3.org/TR/page-visibility/) is supported, the carousel will avoid sliding when the webpage is not visible to the user (such as when the browser tab is inactive, the browser window is minimized, etc.). -The carousel supports swipe gestures (left and right) using [HammerJS]({{ site.cdn.hammer }}). For this to function correctly you need to include HammerJS before Bootstrap or use `bootstrap.bundle.min.js` / `bootstrap.bundle.js` which contains HammerJS. - Please be aware that nested carousels are not supported, and carousels are generally not compliant with accessibility standards. Lastly, if you're building our JavaScript from source, it [requires `util.js`]({{ site.baseurl }}/docs/{{ site.docs_version }}/getting-started/javascript/#util). @@ -287,7 +285,7 @@ Options can be passed via data attributes or JavaScript. For data attributes, ap touch boolean true - Whether the carousel should handle touch event and allow swipe left/right. + Whether the carousel should support left/right swipe interactions on touchscreen devices. diff --git a/site/docs/4.1/getting-started/contents.md b/site/docs/4.1/getting-started/contents.md index 0c210d0438aa..fd38e2fba5e9 100644 --- a/site/docs/4.1/getting-started/contents.md +++ b/site/docs/4.1/getting-started/contents.md @@ -38,7 +38,7 @@ bootstrap/ └── bootstrap.min.js.map {% endhighlight %} -This is the most basic form of Bootstrap: precompiled files for quick drop-in usage in nearly any web project. We provide compiled CSS and JS (`bootstrap.*`), as well as compiled and minified CSS and JS (`bootstrap.min.*`). [source maps](https://developers.google.com/web/tools/chrome-devtools/javascript/source-maps) (`bootstrap.*.map`) are available for use with certain browsers' developer tools. Bundled JS files (`bootstrap.bundle.js` and minified `bootstrap.bundle.min.js`) include [Popper](https://popper.js.org/) and [HammerJS](https://hammerjs.github.io/), but not [jQuery](https://jquery.com/). +This is the most basic form of Bootstrap: precompiled files for quick drop-in usage in nearly any web project. We provide compiled CSS and JS (`bootstrap.*`), as well as compiled and minified CSS and JS (`bootstrap.min.*`). [source maps](https://developers.google.com/web/tools/chrome-devtools/javascript/source-maps) (`bootstrap.*.map`) are available for use with certain browsers' developer tools. Bundled JS files (`bootstrap.bundle.js` and minified `bootstrap.bundle.min.js`) include [Popper](https://popper.js.org/), but not [jQuery](https://jquery.com/). ## CSS files diff --git a/site/docs/4.1/getting-started/introduction.md b/site/docs/4.1/getting-started/introduction.md index 0ad797301050..55ced2f9c318 100644 --- a/site/docs/4.1/getting-started/introduction.md +++ b/site/docs/4.1/getting-started/introduction.md @@ -37,7 +37,7 @@ We use [jQuery's slim build](https://blog.jquery.com/2016/06/09/jquery-3-0-final Curious which components explicitly require jQuery, our JS, and Popper.js? Click the show components link below. If you're at all unsure about the general page structure, keep reading for an example page template. -Our `bootstrap.bundle.js` and `bootstrap.bundle.min.js` include [Popper](https://popper.js.org/) and [HammerJS](https://hammerjs.github.io/), but not [jQuery](https://jquery.com/). For more information about what's included in Bootstrap, please see our [contents]({{ site.baseurl }}/docs/{{ site.docs_version }}/getting-started/contents/#precompiled-bootstrap) section. +Our `bootstrap.bundle.js` and `bootstrap.bundle.min.js` include [Popper](https://popper.js.org/), but not [jQuery](https://jquery.com/). For more information about what's included in Bootstrap, please see our [contents]({{ site.baseurl }}/docs/{{ site.docs_version }}/getting-started/contents/#precompiled-bootstrap) section.
Show components requiring JavaScript From 204c09fed66ae9f21c176eaa88e1c9376694c67e Mon Sep 17 00:00:00 2001 From: Johann-S Date: Sun, 14 Oct 2018 23:10:13 +0200 Subject: [PATCH 3/9] use pointer events if available --- js/src/carousel.js | 77 +++++++++++++++++------ js/tests/unit/carousel.js | 125 +++++++++++++++++++++++++++++++++++++- package.json | 2 +- scss/_carousel.scss | 4 ++ 4 files changed, 185 insertions(+), 23 deletions(-) diff --git a/js/src/carousel.js b/js/src/carousel.js index 5b6209460a4e..f0ad83bb0b49 100644 --- a/js/src/carousel.js +++ b/js/src/carousel.js @@ -56,22 +56,28 @@ const Event = { KEYDOWN : `keydown${EVENT_KEY}`, MOUSEENTER : `mouseenter${EVENT_KEY}`, MOUSELEAVE : `mouseleave${EVENT_KEY}`, - TOUCHEND : `touchend${EVENT_KEY}`, TOUCHSTART : `touchstart${EVENT_KEY}`, TOUCHMOVE : `touchmove${EVENT_KEY}`, + TOUCHEND : `touchend${EVENT_KEY}`, + POINTERDOWN : `pointerdown${EVENT_KEY}`, + POINTERMOVE : `pointermove${EVENT_KEY}`, + POINTERUP : `pointerup${EVENT_KEY}`, + POINTERLEAVE : `pointerleave${EVENT_KEY}`, + POINTERCANCEL : `pointercancel${EVENT_KEY}`, LOAD_DATA_API : `load${EVENT_KEY}${DATA_API_KEY}`, CLICK_DATA_API : `click${EVENT_KEY}${DATA_API_KEY}` } const ClassName = { - CAROUSEL : 'carousel', - ACTIVE : 'active', - SLIDE : 'slide', - RIGHT : 'carousel-item-right', - LEFT : 'carousel-item-left', - NEXT : 'carousel-item-next', - PREV : 'carousel-item-prev', - ITEM : 'carousel-item' + CAROUSEL : 'carousel', + ACTIVE : 'active', + SLIDE : 'slide', + RIGHT : 'carousel-item-right', + LEFT : 'carousel-item-left', + NEXT : 'carousel-item-next', + PREV : 'carousel-item-prev', + ITEM : 'carousel-item', + POINTER_EVENT : 'pointer-event' } const Selector = { @@ -84,6 +90,11 @@ const Selector = { DATA_RIDE : '[data-ride="carousel"]' } +const PointerType = { + TOUCH : 'touch', + PEN : 'pen' +} + /** * ------------------------------------------------------------------------ * Class Definition @@ -103,7 +114,8 @@ class Carousel { this._config = this._getConfig(config) this._element = element this._indicatorsElement = this._element.querySelector(Selector.INDICATORS) - this._touchSupported = 'ontouchstart' in document.documentElement + this._touchSupported = 'ontouchstart' in document.documentElement || navigator.maxTouchPoints > 0 + this._pointerEvent = Boolean(window.PointerEvent || window.MSPointerEvent) this._addEventListeners() } @@ -265,22 +277,35 @@ class Carousel { return } - $(this._element).on(Event.TOUCHSTART, (event) => { - this.touchStartX = event.originalEvent.touches[0].pageX - }) + const start = (event) => { + event.preventDefault() + const originEvent = event.originalEvent - $(this._element).on(Event.TOUCHMOVE, (event) => { + if (this._pointerEvent && (originEvent.pointerType === PointerType.TOUCH || originEvent.pointerType === PointerType.PEN)) { + this.touchStartX = originEvent.clientX + } else { + this.touchStartX = originEvent.touches[0].pageX + } + } + + const move = (event) => { event.preventDefault() // ensure swiping with one touch and not pinching - if (event.originalEvent.touches.length > 1) { + if (event.originalEvent.touches && event.originalEvent.touches.length > 1) { return } - this.touchDeltaX = event.originalEvent.touches[0].pageX - this.touchStartX - }) + if (!this._pointerEvent) { + this.touchDeltaX = event.originalEvent.touches[0].pageX - this.touchStartX + } + } + + const end = (event) => { + if (this._pointerEvent) { + this.touchDeltaX = event.originalEvent.clientX - this.touchStartX + } - $(this._element).on(Event.TOUCHEND, () => { this._handleSwipe() if (this._config.pause === 'hover') { @@ -298,7 +323,21 @@ class Carousel { } this.touchTimeout = setTimeout((event) => this.cycle(event), TOUCHEVENT_COMPAT_WAIT + this._config.interval) } - }) + } + + if (this._pointerEvent) { + $(this._element).on(Event.POINTERDOWN, (event) => start(event)) + $(this._element).on(Event.POINTERMOVE, (event) => move(event)) + $(this._element).on(Event.POINTERUP, (event) => end(event)) + $(this._element).on(Event.POINTERLEAVE, (event) => end(event)) + $(this._element).on(Event.POINTERCANCEL, (event) => end(event)) + + this._element.classList.add(ClassName.POINTER_EVENT) + } else { + $(this._element).on(Event.TOUCHSTART, (event) => start(event)) + $(this._element).on(Event.TOUCHMOVE, (event) => move(event)) + $(this._element).on(Event.TOUCHEND, (event) => end(event)) + } } _keydown(event) { diff --git a/js/tests/unit/carousel.js b/js/tests/unit/carousel.js index e416ab20e427..68c28aec5e98 100644 --- a/js/tests/unit/carousel.js +++ b/js/tests/unit/carousel.js @@ -3,6 +3,26 @@ $(function () { window.Carousel = typeof bootstrap !== 'undefined' ? bootstrap.Carousel : Carousel + var originWinPointerEvent = window.PointerEvent + var originMsPointerEvent = window.MSPointerEvent + var supportPointerEvent = Boolean(window.PointerEvent || window.MSPointerEvent) + + function clearPointerEvents() { + window.PointerEvent = null + window.MSPointerEvent = null + } + + function restorePointerEvents() { + window.PointerEvent = originWinPointerEvent + window.MSPointerEvent = originMsPointerEvent + } + + var stylesCarousel = [ + '' + ].join('') + QUnit.module('carousel plugin') QUnit.test('should be defined on jQuery object', function (assert) { @@ -1006,8 +1026,55 @@ $(function () { }, 80) }) - QUnit.test('should allow swiperight and call prev', function (assert) { + QUnit.test('should allow swiperight and call prev with pointer events', function (assert) { + if (!supportPointerEvent) { + assert.expect(0) + return + } + + Simulator.setType('pointer') + assert.expect(3) + var $styles = $(stylesCarousel).appendTo('head') + var done = assert.async() + document.documentElement.ontouchstart = $.noop + + var carouselHTML = + '
' + + '
' + + ' ' + + '
' + + ' ' + + '
' + + '
' + + '
' + + var $carousel = $(carouselHTML) + $carousel.appendTo('#qunit-fixture') + var $item = $('#item') + $carousel.bootstrapCarousel() + var carousel = $carousel.data('bs.carousel') + var spy = sinon.spy(carousel, 'prev') + + $carousel.one('slid.bs.carousel', function () { + assert.ok(true, 'slid event fired') + assert.ok($item.hasClass('active')) + assert.ok(spy.called) + delete document.documentElement.ontouchstart + $styles.remove() + done() + }) + + Simulator.gestures.swipe($carousel[0], { + deltaX: 300, + deltaY: 0 + }) + }) + + QUnit.test('should allow swiperight and call prev with touch events', function (assert) { Simulator.setType('touch') + clearPointerEvents() assert.expect(3) var done = assert.async() document.documentElement.ontouchstart = $.noop @@ -1036,6 +1103,7 @@ $(function () { assert.ok($item.hasClass('active')) assert.ok(spy.called) delete document.documentElement.ontouchstart + restorePointerEvents() done() }) @@ -1045,8 +1113,56 @@ $(function () { }) }) - QUnit.test('should allow swipeleft and call next', function (assert) { + QUnit.test('should allow swipeleft and call next with pointer events', function (assert) { + if (!supportPointerEvent) { + assert.expect(0) + return + } + + assert.expect(3) + Simulator.setType('pointer') + + var $styles = $(stylesCarousel).appendTo('head') + var done = assert.async() + document.documentElement.ontouchstart = $.noop + + var carouselHTML = + '
' + + '
' + + ' ' + + '
' + + ' ' + + '
' + + '
' + + '
' + + var $carousel = $(carouselHTML) + $carousel.appendTo('#qunit-fixture') + var $item = $('#item') + $carousel.bootstrapCarousel() + var carousel = $carousel.data('bs.carousel') + var spy = sinon.spy(carousel, 'next') + + $carousel.one('slid.bs.carousel', function () { + assert.ok(true, 'slid event fired') + assert.ok(!$item.hasClass('active')) + assert.ok(spy.called) + $styles.remove() + done() + }) + + Simulator.gestures.swipe($carousel[0], { + pos: [300, 10], + deltaX: -300, + deltaY: 0 + }) + }) + + QUnit.test('should allow swipeleft and call next with touch events', function (assert) { assert.expect(3) + clearPointerEvents() Simulator.setType('touch') var done = assert.async() @@ -1075,6 +1191,7 @@ $(function () { assert.ok(true, 'slid event fired') assert.ok(!$item.hasClass('active')) assert.ok(spy.called) + restorePointerEvents() done() }) @@ -1085,8 +1202,9 @@ $(function () { }) }) - QUnit.test('should not allow pinch', function (assert) { + QUnit.test('should not allow pinch with touch events', function (assert) { assert.expect(0) + clearPointerEvents() Simulator.setType('touch') var done = assert.async() document.documentElement.ontouchstart = $.noop @@ -1102,6 +1220,7 @@ $(function () { deltaY: 0, touches: 2 }, function () { + restorePointerEvents() done() }) }) diff --git a/package.json b/package.json index f9b60f13ef8e..5d233ae803cf 100644 --- a/package.json +++ b/package.json @@ -189,7 +189,7 @@ }, { "path": "./dist/js/bootstrap.js", - "maxSize": "21 kB" + "maxSize": "22 kB" }, { "path": "./dist/js/bootstrap.min.js", diff --git a/scss/_carousel.scss b/scss/_carousel.scss index 97e792ea722d..7d3728b40ded 100644 --- a/scss/_carousel.scss +++ b/scss/_carousel.scss @@ -12,6 +12,10 @@ position: relative; } +.carousel.pointer-event { + touch-action: pan-x; +} + .carousel-inner { position: relative; width: 100%; From e44bc863d2eb9464c75825e73bdc0d0b7b038706 Mon Sep 17 00:00:00 2001 From: patrickhlauke Date: Mon, 15 Oct 2018 20:53:51 +0100 Subject: [PATCH 4/9] Use correct touch-action values - my fault, my original advice of using `touch-action: pan-x` is exactly the value we *don't* want to have the browser handle... --- js/tests/unit/carousel.js | 2 +- scss/_carousel.scss | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/js/tests/unit/carousel.js b/js/tests/unit/carousel.js index 68c28aec5e98..932b90f7dd24 100644 --- a/js/tests/unit/carousel.js +++ b/js/tests/unit/carousel.js @@ -19,7 +19,7 @@ $(function () { var stylesCarousel = [ '' ].join('') diff --git a/scss/_carousel.scss b/scss/_carousel.scss index 7d3728b40ded..130965f7985a 100644 --- a/scss/_carousel.scss +++ b/scss/_carousel.scss @@ -13,7 +13,7 @@ } .carousel.pointer-event { - touch-action: pan-x; + touch-action: pan-y pinch-zoom; } .carousel-inner { From feaff43b58d1c4011b6c39a2ba932228dd0eabb4 Mon Sep 17 00:00:00 2001 From: patrickhlauke Date: Mon, 15 Oct 2018 21:54:20 +0100 Subject: [PATCH 5/9] Abandon swipe altogether if more than one touch detected --- js/src/carousel.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/js/src/carousel.js b/js/src/carousel.js index f0ad83bb0b49..997424bfac9e 100644 --- a/js/src/carousel.js +++ b/js/src/carousel.js @@ -283,8 +283,8 @@ class Carousel { if (this._pointerEvent && (originEvent.pointerType === PointerType.TOUCH || originEvent.pointerType === PointerType.PEN)) { this.touchStartX = originEvent.clientX - } else { - this.touchStartX = originEvent.touches[0].pageX + } else if (!this._pointerEvent) { + this.touchStartX = originEvent.touches[0].clientX } } @@ -293,11 +293,12 @@ class Carousel { // ensure swiping with one touch and not pinching if (event.originalEvent.touches && event.originalEvent.touches.length > 1) { + this.touchDeltaX = 0; return } if (!this._pointerEvent) { - this.touchDeltaX = event.originalEvent.touches[0].pageX - this.touchStartX + this.touchDeltaX = event.originalEvent.touches[0].clientX - this.touchStartX } } From f15b98e187e474300ecd8fe400fd4541544c41a8 Mon Sep 17 00:00:00 2001 From: patrickhlauke Date: Mon, 15 Oct 2018 22:29:47 +0100 Subject: [PATCH 6/9] Remove unnecessary pointer event listeners these may also be the cause of weird behavior in Chrome/Surface, where scrolling vertically triggers slide advance --- js/src/carousel.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/js/src/carousel.js b/js/src/carousel.js index 997424bfac9e..797d84bb03b6 100644 --- a/js/src/carousel.js +++ b/js/src/carousel.js @@ -328,9 +328,7 @@ class Carousel { if (this._pointerEvent) { $(this._element).on(Event.POINTERDOWN, (event) => start(event)) - $(this._element).on(Event.POINTERMOVE, (event) => move(event)) $(this._element).on(Event.POINTERUP, (event) => end(event)) - $(this._element).on(Event.POINTERLEAVE, (event) => end(event)) $(this._element).on(Event.POINTERCANCEL, (event) => end(event)) this._element.classList.add(ClassName.POINTER_EVENT) From 20985956e4608bd37de44fe3f4050a6cbe0a5b3d Mon Sep 17 00:00:00 2001 From: patrickhlauke Date: Mon, 15 Oct 2018 22:32:17 +0100 Subject: [PATCH 7/9] Refactor (and correct) start/move/end functions in particular, no need to use originEvent, and preventDefault() only needed for touch events --- js/src/carousel.js | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/js/src/carousel.js b/js/src/carousel.js index 797d84bb03b6..3507a8159483 100644 --- a/js/src/carousel.js +++ b/js/src/carousel.js @@ -278,32 +278,29 @@ class Carousel { } const start = (event) => { - event.preventDefault() - const originEvent = event.originalEvent - - if (this._pointerEvent && (originEvent.pointerType === PointerType.TOUCH || originEvent.pointerType === PointerType.PEN)) { - this.touchStartX = originEvent.clientX + if (this._pointerEvent && (event.originalEvent.pointerType === PointerType.TOUCH || event.originalEvent.pointerType === PointerType.PEN)) { + this.touchStartX = event.originalEvent.clientX } else if (!this._pointerEvent) { - this.touchStartX = originEvent.touches[0].clientX + event.preventDefault() + this.touchStartX = event.originalEvent.touches[0].clientX } } const move = (event) => { - event.preventDefault() - - // ensure swiping with one touch and not pinching - if (event.originalEvent.touches && event.originalEvent.touches.length > 1) { - this.touchDeltaX = 0; - return - } - if (!this._pointerEvent) { - this.touchDeltaX = event.originalEvent.touches[0].clientX - this.touchStartX + event.preventDefault() + + // ensure swiping with one touch and not pinching + if (event.originalEvent.touches && event.originalEvent.touches.length > 1) { + this.touchDeltaX = 0 + } else { + this.touchDeltaX = event.originalEvent.touches[0].clientX - this.touchStartX + } } } const end = (event) => { - if (this._pointerEvent) { + if (this._pointerEvent && (event.originalEvent.pointerType === PointerType.TOUCH || event.originalEvent.pointerType === PointerType.PEN)) { this.touchDeltaX = event.originalEvent.clientX - this.touchStartX } @@ -329,7 +326,6 @@ class Carousel { if (this._pointerEvent) { $(this._element).on(Event.POINTERDOWN, (event) => start(event)) $(this._element).on(Event.POINTERUP, (event) => end(event)) - $(this._element).on(Event.POINTERCANCEL, (event) => end(event)) this._element.classList.add(ClassName.POINTER_EVENT) } else { From 49f8f23419f9dfbc46b991961791c76d8e461e62 Mon Sep 17 00:00:00 2001 From: patrickhlauke Date: Mon, 15 Oct 2018 23:25:44 +0100 Subject: [PATCH 8/9] Set touch-action to "none" Firefox currently seems extremely fickle - with `pan-y` if fires pointercancel as soon as a touch strays even a pixel or so vertically. While `touch-action: pan-y` would be ideal (allowing users to scroll the page even when their finger started the scroll on the carousel), this prevents a swipe that isn't perfectly/only horizontal to be recognised by Firefox. --- js/tests/unit/carousel.js | 2 +- scss/_carousel.scss | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/js/tests/unit/carousel.js b/js/tests/unit/carousel.js index 932b90f7dd24..bafd45fe78d8 100644 --- a/js/tests/unit/carousel.js +++ b/js/tests/unit/carousel.js @@ -19,7 +19,7 @@ $(function () { var stylesCarousel = [ '' ].join('') diff --git a/scss/_carousel.scss b/scss/_carousel.scss index 130965f7985a..41d4e60265bc 100644 --- a/scss/_carousel.scss +++ b/scss/_carousel.scss @@ -1,11 +1,14 @@ // Notes on the classes: // -// 1. The .carousel-item-left and .carousel-item-right is used to indicate where +// 1. .carousel.pointer-event should ideally be pan-y (to allow for users to scroll vertically) +// even when their scroll action started on a carousel, but for compatibility (with Firefox) +// we're preventing all actions instead +// 2. The .carousel-item-left and .carousel-item-right is used to indicate where // the active slide is heading. -// 2. .active.carousel-item is the current slide. -// 3. .active.carousel-item-left and .active.carousel-item-right is the current +// 3. .active.carousel-item is the current slide. +// 4. .active.carousel-item-left and .active.carousel-item-right is the current // slide in its in-transition state. Only one of these occurs at a time. -// 4. .carousel-item-next.carousel-item-left and .carousel-item-prev.carousel-item-right +// 5. .carousel-item-next.carousel-item-left and .carousel-item-prev.carousel-item-right // is the upcoming slide in transition. .carousel { @@ -13,7 +16,7 @@ } .carousel.pointer-event { - touch-action: pan-y pinch-zoom; + touch-action: none; } .carousel-inner { From 098d089dae063aa5253484de60590895e273cfd6 Mon Sep 17 00:00:00 2001 From: Johann-S Date: Tue, 16 Oct 2018 11:10:29 +0200 Subject: [PATCH 9/9] avoid drag img --- js/src/carousel.js | 3 +++ js/tests/unit/carousel.js | 20 +++++++++++--------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/js/src/carousel.js b/js/src/carousel.js index 3507a8159483..989390aa0a46 100644 --- a/js/src/carousel.js +++ b/js/src/carousel.js @@ -64,6 +64,7 @@ const Event = { POINTERUP : `pointerup${EVENT_KEY}`, POINTERLEAVE : `pointerleave${EVENT_KEY}`, POINTERCANCEL : `pointercancel${EVENT_KEY}`, + DRAG_START : `dragstart${EVENT_KEY}`, LOAD_DATA_API : `load${EVENT_KEY}${DATA_API_KEY}`, CLICK_DATA_API : `click${EVENT_KEY}${DATA_API_KEY}` } @@ -84,6 +85,7 @@ const Selector = { ACTIVE : '.active', ACTIVE_ITEM : '.active.carousel-item', ITEM : '.carousel-item', + ITEM_IMG : '.carousel-item img', NEXT_PREV : '.carousel-item-next, .carousel-item-prev', INDICATORS : '.carousel-indicators', DATA_SLIDE : '[data-slide], [data-slide-to]', @@ -323,6 +325,7 @@ class Carousel { } } + $(this._element.querySelectorAll(Selector.ITEM_IMG)).on(Event.DRAG_START, (e) => e.preventDefault()) if (this._pointerEvent) { $(this._element).on(Event.POINTERDOWN, (event) => start(event)) $(this._element).on(Event.POINTERUP, (event) => end(event)) diff --git a/js/tests/unit/carousel.js b/js/tests/unit/carousel.js index bafd45fe78d8..d7d9ad250867 100644 --- a/js/tests/unit/carousel.js +++ b/js/tests/unit/carousel.js @@ -4,17 +4,15 @@ $(function () { window.Carousel = typeof bootstrap !== 'undefined' ? bootstrap.Carousel : Carousel var originWinPointerEvent = window.PointerEvent - var originMsPointerEvent = window.MSPointerEvent + window.MSPointerEvent = null var supportPointerEvent = Boolean(window.PointerEvent || window.MSPointerEvent) function clearPointerEvents() { window.PointerEvent = null - window.MSPointerEvent = null } function restorePointerEvents() { window.PointerEvent = originWinPointerEvent - window.MSPointerEvent = originMsPointerEvent } var stylesCarousel = [ @@ -1032,11 +1030,11 @@ $(function () { return } + document.documentElement.ontouchstart = $.noop Simulator.setType('pointer') assert.expect(3) var $styles = $(stylesCarousel).appendTo('head') var done = assert.async() - document.documentElement.ontouchstart = $.noop var carouselHTML = '
' + @@ -1050,8 +1048,7 @@ $(function () { '
' + '' - var $carousel = $(carouselHTML) - $carousel.appendTo('#qunit-fixture') + var $carousel = $(carouselHTML).appendTo('#qunit-fixture') var $item = $('#item') $carousel.bootstrapCarousel() var carousel = $carousel.data('bs.carousel') @@ -1061,8 +1058,8 @@ $(function () { assert.ok(true, 'slid event fired') assert.ok($item.hasClass('active')) assert.ok(spy.called) - delete document.documentElement.ontouchstart $styles.remove() + delete document.documentElement.ontouchstart done() }) @@ -1075,6 +1072,7 @@ $(function () { QUnit.test('should allow swiperight and call prev with touch events', function (assert) { Simulator.setType('touch') clearPointerEvents() + assert.expect(3) var done = assert.async() document.documentElement.ontouchstart = $.noop @@ -1119,12 +1117,12 @@ $(function () { return } + document.documentElement.ontouchstart = $.noop assert.expect(3) Simulator.setType('pointer') var $styles = $(stylesCarousel).appendTo('head') var done = assert.async() - document.documentElement.ontouchstart = $.noop var carouselHTML = '
' + @@ -1150,6 +1148,7 @@ $(function () { assert.ok(!$item.hasClass('active')) assert.ok(spy.called) $styles.remove() + delete document.documentElement.ontouchstart done() }) @@ -1164,9 +1163,9 @@ $(function () { assert.expect(3) clearPointerEvents() Simulator.setType('touch') + document.documentElement.ontouchstart = $.noop var done = assert.async() - document.documentElement.ontouchstart = $.noop var carouselHTML = '
' + @@ -1192,6 +1191,7 @@ $(function () { assert.ok(!$item.hasClass('active')) assert.ok(spy.called) restorePointerEvents() + delete document.documentElement.ontouchstart done() }) @@ -1205,6 +1205,7 @@ $(function () { QUnit.test('should not allow pinch with touch events', function (assert) { assert.expect(0) clearPointerEvents() + Simulator.setType('touch') var done = assert.async() document.documentElement.ontouchstart = $.noop @@ -1221,6 +1222,7 @@ $(function () { touches: 2 }, function () { restorePointerEvents() + delete document.documentElement.ontouchstart done() }) })