From 22726d8d3bd7760cd225c72f5616e6e9d98a9ba6 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 21 Apr 2017 16:45:41 +0200 Subject: [PATCH 1/4] FocusCycler#focusNext/Previous should focus first/last focusable when no view is currently focused. --- src/focuscycler.js | 10 ++++++++-- tests/focuscycler.js | 8 ++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/focuscycler.js b/src/focuscycler.js index 716be27d..992e942a 100644 --- a/src/focuscycler.js +++ b/src/focuscycler.js @@ -241,13 +241,19 @@ export default class FocusCycler { */ _getFocusableItem( step ) { // Cache for speed. - const current = this.current; + let current = this.current; const collectionLength = this.focusables.length; - if ( !collectionLength || current === null ) { + if ( !collectionLength ) { return null; } + // Start from the beginning if no view is focused. + // https://github.com/ckeditor/ckeditor5-ui/issues/206 + if ( current === null ) { + return this[ step === 1 ? 'first' : 'last' ]; + } + // Cycle in both directions. let index = ( current + collectionLength + step ) % collectionLength; diff --git a/tests/focuscycler.js b/tests/focuscycler.js index 330fdc71..ac0add73 100644 --- a/tests/focuscycler.js +++ b/tests/focuscycler.js @@ -112,10 +112,10 @@ describe( 'FocusCycler', () => { expect( cycler.next ).to.equal( focusables.get( 2 ) ); } ); - it( 'returns null when no view is focused', () => { + it( 'focuses the first focusable view when no view is focused', () => { focusTracker.focusedElement = null; - expect( cycler.next ).to.be.null; + expect( cycler.next ).to.equal( focusables.get( 1 ) ); } ); it( 'returns null when no items', () => { @@ -162,10 +162,10 @@ describe( 'FocusCycler', () => { expect( cycler.previous ).to.equal( focusables.get( 2 ) ); } ); - it( 'returns null when no view is focused', () => { + it( 'focuses the last focusable view when no view is focused', () => { focusTracker.focusedElement = null; - expect( cycler.previous ).to.be.null; + expect( cycler.previous ).to.equal( focusables.get( 3 ) ); } ); it( 'returns null when no items', () => { From d269cd8f461f11c36fb914bd73d51052bc0200ed Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 21 Apr 2017 16:47:40 +0200 Subject: [PATCH 2/4] Created preventDefault() utility that prevents default of native DOM events if event target equals view#element. --- src/bindings/preventdefault.js | 23 ++++++++++++ tests/bindings/preventdefault.js | 63 ++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 src/bindings/preventdefault.js create mode 100644 tests/bindings/preventdefault.js diff --git a/src/bindings/preventdefault.js b/src/bindings/preventdefault.js new file mode 100644 index 00000000..7182bb7b --- /dev/null +++ b/src/bindings/preventdefault.js @@ -0,0 +1,23 @@ +/** + * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module ui/bindings/preventdefault + */ + +/** + * Returns a {module:ui/template~TemplateToBinding} resulting in a native `event#preventDefault` + * for the DOM event if `event#target` equals {@link module:ui/view~View#element}. + * + * @param {module:ui/view~View} view View instance that uses the template. + * @returns {module:ui/template~TemplateToBinding} + */ +export default function preventDefault( view ) { + return view.bindTemplate.to( evt => { + if ( evt.target === view.element ) { + evt.preventDefault(); + } + } ); +} diff --git a/tests/bindings/preventdefault.js b/tests/bindings/preventdefault.js new file mode 100644 index 00000000..22305bf6 --- /dev/null +++ b/tests/bindings/preventdefault.js @@ -0,0 +1,63 @@ +/** + * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* global Event */ + +import preventDefault from '../../src/bindings/preventdefault'; +import View from '../../src/view'; +import Template from '../../src/template'; + +describe( 'preventDefault', () => { + it( 'prevents default of a native DOM event', () => { + const view = new View(); + + view.template = new Template( { + tag: 'div', + + on: { + foo: preventDefault( view ) + } + } ); + + const evt = new Event( 'foo', { bubbles: true } ); + const spy = sinon.spy( evt, 'preventDefault' ); + + // Render to enable bubbling. + view.element; + + view.element.dispatchEvent( evt ); + sinon.assert.calledOnce( spy ); + } ); + + it( 'prevents only when target is view#element', () => { + const view = new View(); + const child = new View(); + + child.template = new Template( { + tag: 'a' + } ); + + view.template = new Template( { + tag: 'div', + + on: { + foo: preventDefault( view ) + }, + + children: [ + child + ] + } ); + + const evt = new Event( 'foo', { bubbles: true } ); + const spy = sinon.spy( evt, 'preventDefault' ); + + // Render to enable bubbling. + view.element; + + child.element.dispatchEvent( evt ); + sinon.assert.notCalled( spy ); + } ); +} ); From 4f6e68bfb051038c198ad14b86ef5c52be689299 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 21 Apr 2017 16:48:02 +0200 Subject: [PATCH 3/4] Fix: BalloonPanelView should not be focusable. --- src/panel/balloon/balloonpanelview.js | 13 ++++++++----- tests/panel/balloon/balloonpanelview.js | 11 ++++++++++- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index ef4d0a88..a4c3f694 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -13,6 +13,7 @@ import { getOptimalPosition } from '@ckeditor/ckeditor5-utils/src/dom/position'; import isRange from '@ckeditor/ckeditor5-utils/src/dom/isrange'; import isElement from '@ckeditor/ckeditor5-utils/src/lib/lodash/isElement'; import toUnit from '@ckeditor/ckeditor5-utils/src/dom/tounit'; +import preventDefault from '../../bindings/preventdefault.js'; import global from '@ckeditor/ckeditor5-utils/src/dom/global'; const toPx = toUnit( 'px' ); @@ -134,13 +135,15 @@ export default class BalloonPanelView extends View { top: bind.to( 'top', toPx ), left: bind.to( 'left', toPx ), maxWidth: bind.to( 'maxWidth', toPx ) - }, - - // Make this element `focusable` to be available for adding to FocusTracker. - tabindex: -1 + } }, - children: this.content + children: this.content, + + on: { + // https://github.com/ckeditor/ckeditor5-ui/issues/206 + mousedown: preventDefault( this ) + } } ); } diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 35ee09a6..5783b796 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -35,7 +35,6 @@ describe( 'BalloonPanelView', () => { it( 'should create element from template', () => { expect( view.element.tagName ).to.equal( 'DIV' ); expect( view.element.classList.contains( 'ck-balloon-panel' ) ).to.true; - expect( view.element.getAttribute( 'tabindex' ) ).to.equal( '-1' ); } ); it( 'should set default values', () => { @@ -129,6 +128,16 @@ describe( 'BalloonPanelView', () => { } ); } ); } ); + + describe( 'event listeners', () => { + it( 'prevent default on #mousedown', () => { + const evt = new Event( 'mousedown', { bubbles: true } ); + const spy = sinon.spy( evt, 'preventDefault' ); + + view.element.dispatchEvent( evt ); + sinon.assert.calledOnce( spy ); + } ); + } ); } ); describe( 'show()', () => { From 270a1a4ce1b496e260b7a070a7dbab385734e40d Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 21 Apr 2017 16:48:12 +0200 Subject: [PATCH 4/4] Fix: ToolbarView should not be focusable. --- src/toolbar/toolbarview.js | 8 +++++++- tests/toolbar/toolbarview.js | 12 +++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/toolbar/toolbarview.js b/src/toolbar/toolbarview.js index 644f5432..e08c1484 100644 --- a/src/toolbar/toolbarview.js +++ b/src/toolbar/toolbarview.js @@ -13,6 +13,7 @@ import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; import FocusCycler from '../focuscycler'; import KeystrokeHandler from '@ckeditor/ckeditor5-utils/src/keystrokehandler'; import ToolbarSeparatorView from './toolbarseparatorview'; +import preventDefault from '../bindings/preventdefault.js'; /** * The toolbar view class. @@ -78,7 +79,12 @@ export default class ToolbarView extends View { ] }, - children: this.items + children: this.items, + + on: { + // https://github.com/ckeditor/ckeditor5-ui/issues/206 + mousedown: preventDefault( this ) + } } ); this.items.on( 'add', ( evt, item ) => { diff --git a/tests/toolbar/toolbarview.js b/tests/toolbar/toolbarview.js index f5811b8c..632d943e 100644 --- a/tests/toolbar/toolbarview.js +++ b/tests/toolbar/toolbarview.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md. */ -/* global document */ +/* global document, Event */ import ToolbarView from '../../src/toolbar/toolbarview'; import ToolbarSeparatorView from '../../src/toolbar/toolbarseparatorview'; @@ -64,6 +64,16 @@ describe( 'ToolbarView', () => { it( 'should create element from template', () => { expect( view.element.classList.contains( 'ck-toolbar' ) ).to.true; } ); + + describe( 'event listeners', () => { + it( 'prevent default on #mousedown', () => { + const evt = new Event( 'mousedown', { bubbles: true } ); + const spy = sinon.spy( evt, 'preventDefault' ); + + view.element.dispatchEvent( evt ); + sinon.assert.calledOnce( spy ); + } ); + } ); } ); describe( 'init()', () => {