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.

Sign up for GitHub

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

Jump to bottom

Carousel: Only prevents default for ARROW_LEFT and ARROW_RIGHT keys #20907

Merged

bardiharborow merged 1 commit into twbs:v4-dev from matheusazzi:carousel-keydowns-prevent-default

Dec 23, 2016

Contributor

matheusazzi commented Oct 16, 2016

Fixes 2 bugs:

All keydowns were being prevented. Because of that the user wasn't able to navigate in the whole page using ARROW_UP/ARROW_DOWN.
Even when is an input or textarea the keydowns were being prevented. Because of that the user wasn't able to type any text on these elements.


          Carousel: Only prevents default for ARROW_LEFT and ARROW_RIGHT keys

74eb964

Fixes 2 bugs:

1. All keydowns were being prevented. Because of that the user wasn't able to navigate in the whole page using ARROW_UP/ARROW_DOWN.

2. Even when  is an input or textarea the keydowns were being prevented. Because of that the user wasn't able to type any text on these elements.

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js


		$template.trigger(eventArrowDown)

		$template.one('keydown', function (event) {

houndci-bot Oct 16, 2016

Unexpected function expression prefer-arrow-callback

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js


		$template.trigger(eventArrowDown)

		$template.one('keydown', function (event) {

houndci-bot Oct 16, 2016

Unexpected function expression prefer-arrow-callback

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

+                  var eventArrowDown = $.Event('keydown', { which: 40 })
+                  var eventArrowUp   = $.Event('keydown', { which: 38 })
+                  $template.one('keydown', function (event) {

houndci-bot Oct 16, 2016

Unexpected function expression prefer-arrow-callback

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

+                  var eventArrowDown = $.Event('keydown', { which: 40 })
+                  var eventArrowUp   = $.Event('keydown', { which: 38 })
+                  $template.one('keydown', function (event) {

houndci-bot Oct 16, 2016

Unexpected function expression prefer-arrow-callback

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

+                  var done = assert.async()
+                  var eventArrowDown = $.Event('keydown', { which: 40 })
+                  var eventArrowUp   = $.Event('keydown', { which: 38 })

houndci-bot Oct 16, 2016

Unexpected var, use let or const instead no-var

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

+                  var done = assert.async()
+                  var eventArrowDown = $.Event('keydown', { which: 40 })
+                  var eventArrowUp   = $.Event('keydown', { which: 38 })

houndci-bot Oct 16, 2016

Unexpected var, use let or const instead no-var

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

+                  $template.bootstrapCarousel()
+                  var done = assert.async()
+                  var eventArrowDown = $.Event('keydown', { which: 40 })

houndci-bot Oct 16, 2016

Unexpected var, use let or const instead no-var

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

+                  $template.bootstrapCarousel()
+                  var done = assert.async()
+                  var eventArrowDown = $.Event('keydown', { which: 40 })

houndci-bot Oct 16, 2016

Unexpected var, use let or const instead no-var

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

+                  var $template = $(templateHTML)
+                  $template.bootstrapCarousel()
+                  var done = assert.async()

houndci-bot Oct 16, 2016

Unexpected var, use let or const instead no-var

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

+                  var $template = $(templateHTML)
+                  $template.bootstrapCarousel()
+                  var done = assert.async()

houndci-bot Oct 16, 2016

Unexpected var, use let or const instead no-var

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

+                      + '</div>'
+                      + '</div>'
+                      + '</div>'
+                  var $template = $(templateHTML)

houndci-bot Oct 16, 2016

Unexpected var, use let or const instead no-var

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

+                      + '</div>'
+                      + '</div>'
+                      + '</div>'
+                  var $template = $(templateHTML)

houndci-bot Oct 16, 2016

Unexpected var, use let or const instead no-var

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

@@ @@ -507,6 +507,37 @@ $(function () { @@
                   assert.strictEqual($template.find('.carousel-item')[1], $template.find('.active')[0], 'second item active')
                 })
+                QUnit.test('should not prevent keydown if key is not ARROW_LEFT or ARROW_RIGHT', function (assert) {
+                  assert.expect(2)
+                  var templateHTML = '<div id="myCarousel" class="carousel" data-interval="false">'

houndci-bot Oct 16, 2016

Unexpected var, use let or const instead no-var

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

@@ @@ -507,6 +507,37 @@ $(function () { @@
                   assert.strictEqual($template.find('.carousel-item')[1], $template.find('.active')[0], 'second item active')
                 })
+                QUnit.test('should not prevent keydown if key is not ARROW_LEFT or ARROW_RIGHT', function (assert) {
+                  assert.expect(2)
+                  var templateHTML = '<div id="myCarousel" class="carousel" data-interval="false">'

houndci-bot Oct 16, 2016

Unexpected var, use let or const instead no-var

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

@@ @@ -507,6 +507,37 @@ $(function () { @@
                   assert.strictEqual($template.find('.carousel-item')[1], $template.find('.active')[0], 'second item active')
                 })
+                QUnit.test('should not prevent keydown if key is not ARROW_LEFT or ARROW_RIGHT', function (assert) {
+                  assert.expect(2)

houndci-bot Oct 16, 2016

No magic number: 2 no-magic-numbers

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

@@ @@ -507,6 +507,37 @@ $(function () { @@
                   assert.strictEqual($template.find('.carousel-item')[1], $template.find('.active')[0], 'second item active')
                 })
+                QUnit.test('should not prevent keydown if key is not ARROW_LEFT or ARROW_RIGHT', function (assert) {
+                  assert.expect(2)

houndci-bot Oct 16, 2016

No magic number: 2 no-magic-numbers

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

@@ @@ -507,6 +507,37 @@ $(function () { @@
                   assert.strictEqual($template.find('.carousel-item')[1], $template.find('.active')[0], 'second item active')
                 })
+                QUnit.test('should not prevent keydown if key is not ARROW_LEFT or ARROW_RIGHT', function (assert) {

houndci-bot Oct 16, 2016

'QUnit' is not defined no-undef
Unexpected function expression prefer-arrow-callback

houndci-bot reviewed

View reviewed changes

js/tests/unit/carousel.js

@@ @@ -507,6 +507,37 @@ $(function () { @@
                   assert.strictEqual($template.find('.carousel-item')[1], $template.find('.active')[0], 'second item active')
                 })
+                QUnit.test('should not prevent keydown if key is not ARROW_LEFT or ARROW_RIGHT', function (assert) {

houndci-bot Oct 16, 2016

'QUnit' is not defined no-undef
Unexpected function expression prefer-arrow-callback

Member

Johann-S commented Oct 16, 2016

Second bug fix already done : #19058

mdo mentioned this pull request

bootrap carousel (keyboard: false) and modal pop up do not mix well #20793

Closed

mdo added js v4 labels

mdo requested a review from bardiharborow

December 23, 2016 05:57

bardiharborow approved these changes

View reviewed changes

bardiharborow merged commit dab6a41 into twbs:v4-dev

bardiharborow added this to the v4.0.0-alpha.6 milestone

mdo mentioned this pull request

v4 Alpha 6 ship list #20939

Closed

Johann-S mentioned this pull request

Unit Test for "Carousel: Only prevents default for ARROW_LEFT and ARROW_RIGHT keys" failed #21412

Closed

bardiharborow mentioned this pull request

Closes: #21412: Fix unit test for carousel #21413

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels