From 9ca56b74da3775bea5f4c14476f9825b74b0b11a Mon Sep 17 00:00:00 2001 From: Mateusz Samsel Date: Fri, 5 Jul 2019 11:38:09 +0200 Subject: [PATCH 1/7] Fix aria-pressed status for not pressed buttons. --- src/button/buttonview.js | 2 +- tests/button/buttonview.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/button/buttonview.js b/src/button/buttonview.js index de86b701..53d01926 100644 --- a/src/button/buttonview.js +++ b/src/button/buttonview.js @@ -132,7 +132,7 @@ export default class ButtonView extends View { tabindex: bind.to( 'tabindex' ), 'aria-labelledby': `ck-editor__aria-label_${ ariaLabelUid }`, 'aria-disabled': bind.if( 'isEnabled', true, value => !value ), - 'aria-pressed': bind.if( 'isOn', true ) + 'aria-pressed': bind.to( 'isOn', value => value ? 'true' : 'false' ) }, children: this.children, diff --git a/tests/button/buttonview.js b/tests/button/buttonview.js index 7d3c403b..af5cd855 100644 --- a/tests/button/buttonview.js +++ b/tests/button/buttonview.js @@ -249,7 +249,7 @@ describe( 'ButtonView', () => { expect( view.element.attributes[ 'aria-pressed' ].value ).to.equal( 'true' ); view.isOn = false; - expect( view.element.attributes[ 'aria-pressed' ] ).to.be.undefined; + expect( view.element.attributes[ 'aria-pressed' ].value ).to.equal( 'false' ); } ); } ); From 5edce86e495aecf6899424116088bf1a18b930fa Mon Sep 17 00:00:00 2001 From: Mateusz Samsel Date: Tue, 16 Jul 2019 16:52:43 +0200 Subject: [PATCH 2/7] Add toggle config option. --- src/button/buttonview.js | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/button/buttonview.js b/src/button/buttonview.js index 53d01926..709cc151 100644 --- a/src/button/buttonview.js +++ b/src/button/buttonview.js @@ -37,9 +37,15 @@ import '../../theme/components/button/button.css'; */ export default class ButtonView extends View { /** - * @inheritDoc + * Creates an instance of the {@link module:ui/view~View} class. + * + * Also see {@link #render}. + * + * @param {module:utils/locale~Locale} [locale] The localization services instance. + * @param {Object} [config] additional configuration + * @param {Boolean} [config.toggle=false] set button as a toggle */ - constructor( locale ) { + constructor( locale, { toggle = false } = {} ) { super( locale ); const bind = this.bindTemplate; @@ -131,8 +137,7 @@ export default class ButtonView extends View { type: bind.to( 'type', value => value ? value : 'button' ), tabindex: bind.to( 'tabindex' ), 'aria-labelledby': `ck-editor__aria-label_${ ariaLabelUid }`, - 'aria-disabled': bind.if( 'isEnabled', true, value => !value ), - 'aria-pressed': bind.to( 'isOn', value => value ? 'true' : 'false' ) + 'aria-disabled': bind.if( 'isEnabled', true, value => !value ) }, children: this.children, @@ -155,6 +160,14 @@ export default class ButtonView extends View { } ) } } ); + + if ( toggle ) { + this.extendTemplate( { + attributes: { + 'aria-pressed': bind.to( 'isOn', value => value ? 'true' : 'false' ) + } + } ); + } } /** From 0a2990a9ed26df6156a2068b8d6ad54c4cc2d3a5 Mon Sep 17 00:00:00 2001 From: Mateusz Samsel Date: Fri, 19 Jul 2019 12:14:27 +0200 Subject: [PATCH 3/7] Provide toggleable observable to button interface. Include it in switchbutton constructor. Correct unit test related to chanegs. --- src/button/button.jsdoc | 8 ++++++++ src/button/buttonview.js | 14 ++++---------- src/button/switchbuttonview.js | 9 +++++++++ tests/button/buttonview.js | 9 +++++++++ tests/button/switchbuttonview.js | 4 ++++ 5 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/button/button.jsdoc b/src/button/button.jsdoc index 70f2cb0f..89a80f8a 100644 --- a/src/button/button.jsdoc +++ b/src/button/button.jsdoc @@ -95,6 +95,14 @@ * @member {Boolean} #isVisible */ +/** + * Controls whether the button view is recognized as toggle button for assistive technologies. + * + * @observable + * @default false + * @member {Boolean} #isToggleable + */ + /** * (Optional) Controls whether the label of the button is hidden (e.g. an icon–only button). * diff --git a/src/button/buttonview.js b/src/button/buttonview.js index 709cc151..4596c22c 100644 --- a/src/button/buttonview.js +++ b/src/button/buttonview.js @@ -45,7 +45,7 @@ export default class ButtonView extends View { * @param {Object} [config] additional configuration * @param {Boolean} [config.toggle=false] set button as a toggle */ - constructor( locale, { toggle = false } = {} ) { + constructor( locale ) { super( locale ); const bind = this.bindTemplate; @@ -58,6 +58,7 @@ export default class ButtonView extends View { this.set( 'isEnabled', true ); this.set( 'isOn', false ); this.set( 'isVisible', true ); + this.set( 'isToggleable', false ); this.set( 'keystroke' ); this.set( 'label' ); this.set( 'tabindex', -1 ); @@ -137,7 +138,8 @@ export default class ButtonView extends View { type: bind.to( 'type', value => value ? value : 'button' ), tabindex: bind.to( 'tabindex' ), 'aria-labelledby': `ck-editor__aria-label_${ ariaLabelUid }`, - 'aria-disabled': bind.if( 'isEnabled', true, value => !value ) + 'aria-disabled': bind.if( 'isEnabled', true, value => !value ), + 'aria-pressed': bind.to( 'isOn', value => this.isToggleable ? String( value ) : false ) }, children: this.children, @@ -160,14 +162,6 @@ export default class ButtonView extends View { } ) } } ); - - if ( toggle ) { - this.extendTemplate( { - attributes: { - 'aria-pressed': bind.to( 'isOn', value => value ? 'true' : 'false' ) - } - } ); - } } /** diff --git a/src/button/switchbuttonview.js b/src/button/switchbuttonview.js index f6b593f5..640c35f9 100644 --- a/src/button/switchbuttonview.js +++ b/src/button/switchbuttonview.js @@ -43,6 +43,15 @@ export default class SwitchButtonView extends ButtonView { */ this.toggleSwitchView = this._createToggleView(); + /** + * Controls whether the button view is recognized as toggle button for assistive technologies. + * + * @observable + * @default true + * @member {Boolean} #isToggleable + */ + this.isToggleable = true; + this.extendTemplate( { attributes: { class: 'ck-switchbutton' diff --git a/tests/button/buttonview.js b/tests/button/buttonview.js index af5cd855..413316f3 100644 --- a/tests/button/buttonview.js +++ b/tests/button/buttonview.js @@ -245,12 +245,21 @@ describe( 'ButtonView', () => { } ); it( '-pressed reacts to #isOn', () => { + view.isToggleable = true; view.isOn = true; expect( view.element.attributes[ 'aria-pressed' ].value ).to.equal( 'true' ); view.isOn = false; expect( view.element.attributes[ 'aria-pressed' ].value ).to.equal( 'false' ); } ); + + it( '-pressed is not present for not toggleable button', () => { + view.isOn = true; + expect( view.element.hasAttribute( 'aria-pressed' ) ).to.be.false; + + view.isOn = false; + expect( view.element.hasAttribute( 'aria-pressed' ) ).to.be.false; + } ); } ); describe( 'mousedown event', () => { diff --git a/tests/button/switchbuttonview.js b/tests/button/switchbuttonview.js index 5bf4763b..493d8f50 100644 --- a/tests/button/switchbuttonview.js +++ b/tests/button/switchbuttonview.js @@ -24,6 +24,10 @@ describe( 'SwitchButtonView', () => { it( 'sets CSS class', () => { expect( view.element.classList.contains( 'ck-switchbutton' ) ).to.be.true; } ); + + it( 'sets isToggleable flag to true', () => { + expect( view.isToggleable ).to.be.true; + } ); } ); describe( 'render', () => { From c5ca390b44957e70c8cd8203ad179cc6aa0bbd3f Mon Sep 17 00:00:00 2001 From: Mateusz Samsel Date: Fri, 9 Aug 2019 10:46:31 +0200 Subject: [PATCH 4/7] Restore proper docs description. --- src/button/buttonview.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/button/buttonview.js b/src/button/buttonview.js index 4596c22c..9128888c 100644 --- a/src/button/buttonview.js +++ b/src/button/buttonview.js @@ -37,13 +37,7 @@ import '../../theme/components/button/button.css'; */ export default class ButtonView extends View { /** - * Creates an instance of the {@link module:ui/view~View} class. - * - * Also see {@link #render}. - * - * @param {module:utils/locale~Locale} [locale] The localization services instance. - * @param {Object} [config] additional configuration - * @param {Boolean} [config.toggle=false] set button as a toggle + * @inheritDoc */ constructor( locale ) { super( locale ); From e9d8e3c26f83cae95ac795f9d97c4e0347e693d2 Mon Sep 17 00:00:00 2001 From: Mateusz Samsel Date: Mon, 12 Aug 2019 10:01:11 +0200 Subject: [PATCH 5/7] Set block button as toggleablt, provide option to set split button as toggleable. --- src/dropdown/button/splitbuttonview.js | 2 ++ src/toolbar/block/blockbuttonview.js | 2 ++ tests/dropdown/button/splitbuttonview.js | 8 ++++++++ tests/toolbar/block/blockbuttonview.js | 4 ++++ 4 files changed, 16 insertions(+) diff --git a/src/dropdown/button/splitbuttonview.js b/src/dropdown/button/splitbuttonview.js index 3b454ac5..f8fdf2ba 100644 --- a/src/dropdown/button/splitbuttonview.js +++ b/src/dropdown/button/splitbuttonview.js @@ -50,6 +50,7 @@ export default class SplitButtonView extends View { this.set( 'icon' ); this.set( 'isEnabled', true ); this.set( 'isOn', false ); + this.set( 'isToggleable', false ); this.set( 'isVisible', true ); this.set( 'keystroke' ); this.set( 'label' ); @@ -173,6 +174,7 @@ export default class SplitButtonView extends View { 'icon', 'isEnabled', 'isOn', + 'isToggleable', 'keystroke', 'label', 'tabindex', diff --git a/src/toolbar/block/blockbuttonview.js b/src/toolbar/block/blockbuttonview.js index 29a0e06a..69aeb970 100644 --- a/src/toolbar/block/blockbuttonview.js +++ b/src/toolbar/block/blockbuttonview.js @@ -34,6 +34,8 @@ export default class BlockButtonView extends ButtonView { // Hide button on init. this.isVisible = false; + this.isToggleable = true; + /** * Top offset. * diff --git a/tests/dropdown/button/splitbuttonview.js b/tests/dropdown/button/splitbuttonview.js index 07c2e297..2fcaba5a 100644 --- a/tests/dropdown/button/splitbuttonview.js +++ b/tests/dropdown/button/splitbuttonview.js @@ -30,6 +30,14 @@ describe( 'SplitButtonView', () => { expect( view.actionView.element.classList.contains( 'ck-splitbutton__action' ) ).to.be.true; } ); + it( 'adds isToggleable to view#actionView', () => { + expect( view.actionView.isToggleable ).to.be.false; + + view.isToggleable = true; + + expect( view.actionView.isToggleable ).to.be.true; + } ); + it( 'creates view#arrowView', () => { expect( view.arrowView ).to.be.instanceOf( ButtonView ); expect( view.arrowView.element.classList.contains( 'ck-splitbutton__arrow' ) ).to.be.true; diff --git a/tests/toolbar/block/blockbuttonview.js b/tests/toolbar/block/blockbuttonview.js index efdda72b..1c42e7d1 100644 --- a/tests/toolbar/block/blockbuttonview.js +++ b/tests/toolbar/block/blockbuttonview.js @@ -22,6 +22,10 @@ describe( 'BlockButtonView', () => { expect( view.element.classList.contains( 'ck-block-toolbar-button' ) ).to.be.true; } ); + it( 'should be initialized as toggleable button', () => { + expect( view.isToggleable ).to.be.true; + } ); + describe( 'DOM binding', () => { it( 'should react on `view#top` change', () => { view.top = 0; From d7b20b88ace6bcceb60195e56512c8f3c92be2be Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 12 Aug 2019 15:27:25 +0200 Subject: [PATCH 6/7] Minor docs and tests improvements. --- src/button/button.jsdoc | 2 +- src/button/switchbuttonview.js | 11 ++--------- tests/button/buttonview.js | 2 +- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/button/button.jsdoc b/src/button/button.jsdoc index 89a80f8a..15c58f36 100644 --- a/src/button/button.jsdoc +++ b/src/button/button.jsdoc @@ -96,7 +96,7 @@ */ /** - * Controls whether the button view is recognized as toggle button for assistive technologies. + * Controls whether the button view is a toggle button (two–state) for assistive technologies. * * @observable * @default false diff --git a/src/button/switchbuttonview.js b/src/button/switchbuttonview.js index 640c35f9..aee616ba 100644 --- a/src/button/switchbuttonview.js +++ b/src/button/switchbuttonview.js @@ -35,6 +35,8 @@ export default class SwitchButtonView extends ButtonView { constructor( locale ) { super( locale ); + this.isToggleable = true; + /** * The toggle switch of the button. * @@ -43,15 +45,6 @@ export default class SwitchButtonView extends ButtonView { */ this.toggleSwitchView = this._createToggleView(); - /** - * Controls whether the button view is recognized as toggle button for assistive technologies. - * - * @observable - * @default true - * @member {Boolean} #isToggleable - */ - this.isToggleable = true; - this.extendTemplate( { attributes: { class: 'ck-switchbutton' diff --git a/tests/button/buttonview.js b/tests/button/buttonview.js index 413316f3..de3402f7 100644 --- a/tests/button/buttonview.js +++ b/tests/button/buttonview.js @@ -253,7 +253,7 @@ describe( 'ButtonView', () => { expect( view.element.attributes[ 'aria-pressed' ].value ).to.equal( 'false' ); } ); - it( '-pressed is not present for not toggleable button', () => { + it( '-pressed is not present for non–toggleable button', () => { view.isOn = true; expect( view.element.hasAttribute( 'aria-pressed' ) ).to.be.false; From 8662c0b1bb1cb0c08abd745e38a6ac59246962c7 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 12 Aug 2019 15:38:52 +0200 Subject: [PATCH 7/7] Implemented the aria-expanded attribute support on the SplitButtonView#arrowView element. --- src/dropdown/button/splitbuttonview.js | 4 +++- tests/dropdown/button/splitbuttonview.js | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/dropdown/button/splitbuttonview.js b/src/dropdown/button/splitbuttonview.js index f8fdf2ba..ca4fd171 100644 --- a/src/dropdown/button/splitbuttonview.js +++ b/src/dropdown/button/splitbuttonview.js @@ -204,13 +204,15 @@ export default class SplitButtonView extends View { */ _createArrowView() { const arrowView = new ButtonView(); + const bind = arrowView.bindTemplate; arrowView.icon = dropdownArrowIcon; arrowView.extendTemplate( { attributes: { class: 'ck-splitbutton__arrow', - 'aria-haspopup': true + 'aria-haspopup': true, + 'aria-expanded': bind.to( 'isOn', value => String( value ) ) } } ); diff --git a/tests/dropdown/button/splitbuttonview.js b/tests/dropdown/button/splitbuttonview.js index 2fcaba5a..29c51336 100644 --- a/tests/dropdown/button/splitbuttonview.js +++ b/tests/dropdown/button/splitbuttonview.js @@ -70,6 +70,14 @@ describe( 'SplitButtonView', () => { expect( view.element.classList.contains( 'ck-splitbutton_open' ) ).to.be.false; } ); + it( 'binds arrowView aria-expanded attribute to #isOn', () => { + view.arrowView.isOn = true; + expect( view.arrowView.element.getAttribute( 'aria-expanded' ) ).to.equal( 'true' ); + + view.arrowView.isOn = false; + expect( view.arrowView.element.getAttribute( 'aria-expanded' ) ).to.equal( 'false' ); + } ); + describe( 'activates keyboard navigation for the toolbar', () => { it( 'so "arrowright" on view#arrowView does nothing', () => { const keyEvtData = {