jenkins-bot has submitted this change and it was merged.
Change subject: Make toolbars keyboard-accessible
......................................................................
Make toolbars keyboard-accessible
* Mix in TabIndexedElement into stuff.
* Duplicate mousedown/mouseup/mouseover/mouseout event handling for
keydown/keyup/focus/blur.
* Tweak the magic for More/Fewer tool in ListToolGroup.
This is not perfect (especially the visual display of focus marker
is sometimes very weird), but it is functional.
Bug: T93240
Change-Id: I4ca8c77bdcea8ea8472d013af28a41361bf69065
---
M src/Tool.js
M src/ToolGroup.js
M src/Toolbar.js
M src/toolgroups/ListToolGroup.js
M src/toolgroups/PopupToolGroup.js
5 files changed, 106 insertions(+), 73 deletions(-)
Approvals:
Esanders: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/Tool.js b/src/Tool.js
index 00ffd75..93e8e99 100644
--- a/src/Tool.js
+++ b/src/Tool.js
@@ -6,6 +6,7 @@
* @extends OO.ui.Widget
* @mixins OO.ui.IconElement
* @mixins OO.ui.FlaggedElement
+ * @mixins OO.ui.TabIndexedElement
*
* @constructor
* @param {OO.ui.ToolGroup} toolGroup
@@ -25,10 +26,6 @@
// Parent constructor
OO.ui.Tool.super.call( this, config );
- // Mixin constructors
- OO.ui.IconElement.call( this, config );
- OO.ui.FlaggedElement.call( this, config );
-
// Properties
this.toolGroup = toolGroup;
this.toolbar = this.toolGroup.getToolbar();
@@ -37,6 +34,11 @@
this.$accel = $( '<span>' );
this.$link = $( '<a>' );
this.title = null;
+
+ // Mixin constructors
+ OO.ui.IconElement.call( this, config );
+ OO.ui.FlaggedElement.call( this, config );
+ OO.ui.TabIndexedElement.call( this, $.extend( {}, config, {
$tabIndexed: this.$link } ) );
// Events
this.toolbar.connect( this, { updateState: 'onUpdateState' } );
@@ -54,7 +56,6 @@
this.$link
.addClass( 'oo-ui-tool-link' )
.append( this.$icon, this.$title, this.$accel )
- .attr( 'tabindex', 0 )
.attr( 'role', 'button' );
this.$element
.data( 'oo-ui-tool', this )
@@ -71,6 +72,7 @@
OO.inheritClass( OO.ui.Tool, OO.ui.Widget );
OO.mixinClass( OO.ui.Tool, OO.ui.IconElement );
OO.mixinClass( OO.ui.Tool, OO.ui.FlaggedElement );
+OO.mixinClass( OO.ui.Tool, OO.ui.TabIndexedElement );
/* Events */
diff --git a/src/ToolGroup.js b/src/ToolGroup.js
index 9046dae..eab3cea 100644
--- a/src/ToolGroup.js
+++ b/src/ToolGroup.js
@@ -45,14 +45,18 @@
this.exclude = config.exclude || [];
this.promote = config.promote || [];
this.demote = config.demote || [];
- this.onCapturedMouseUpHandler = this.onCapturedMouseUp.bind( this );
+ this.onCapturedMouseKeyUpHandler = this.onCapturedMouseKeyUp.bind( this
);
// Events
this.$element.on( {
- mousedown: this.onPointerDown.bind( this ),
- mouseup: this.onPointerUp.bind( this ),
- mouseover: this.onMouseOver.bind( this ),
- mouseout: this.onMouseOut.bind( this )
+ mousedown: this.onMouseKeyDown.bind( this ),
+ mouseup: this.onMouseKeyUp.bind( this ),
+ keydown: this.onMouseKeyDown.bind( this ),
+ keyup: this.onMouseKeyUp.bind( this ),
+ focus: this.onMouseOverFocus.bind( this ),
+ blur: this.onMouseOutBlur.bind( this ),
+ mouseover: this.onMouseOverFocus.bind( this ),
+ mouseout: this.onMouseOutBlur.bind( this )
} );
this.toolbar.getToolFactory().connect( this, { register:
'onToolFactoryRegister' } );
this.aggregate( { disable: 'itemDisable' } );
@@ -135,57 +139,64 @@
};
/**
- * Handle mouse down events.
+ * Handle mouse down and key down events.
*
- * @param {jQuery.Event} e Mouse down event
+ * @param {jQuery.Event} e Mouse down or key down event
*/
-OO.ui.ToolGroup.prototype.onPointerDown = function ( e ) {
- if ( !this.isDisabled() && e.which === 1 ) {
+OO.ui.ToolGroup.prototype.onMouseKeyDown = function ( e ) {
+ if (
+ !this.isDisabled() &&
+ ( e.which === 1 || e.which === OO.ui.Keys.SPACE || e.which ===
OO.ui.Keys.ENTER )
+ ) {
this.pressed = this.getTargetTool( e );
if ( this.pressed ) {
this.pressed.setActive( true );
- this.getElementDocument().addEventListener(
- 'mouseup', this.onCapturedMouseUpHandler, true
- );
+ this.getElementDocument().addEventListener( 'mouseup',
this.onCapturedMouseKeyUpHandler, true );
+ this.getElementDocument().addEventListener( 'keyup',
this.onCapturedMouseKeyUpHandler, true );
}
+ return false;
}
- return false;
};
/**
- * Handle captured mouse up events.
+ * Handle captured mouse up and key up events.
*
- * @param {Event} e Mouse up event
+ * @param {Event} e Mouse up or key up event
*/
-OO.ui.ToolGroup.prototype.onCapturedMouseUp = function ( e ) {
- this.getElementDocument().removeEventListener( 'mouseup',
this.onCapturedMouseUpHandler, true );
- // onPointerUp may be called a second time, depending on where the
mouse is when the button is
+OO.ui.ToolGroup.prototype.onCapturedMouseKeyUp = function ( e ) {
+ this.getElementDocument().removeEventListener( 'mouseup',
this.onCapturedMouseKeyUpHandler, true );
+ this.getElementDocument().removeEventListener( 'keyup',
this.onCapturedMouseKeyUpHandler, true );
+ // onMouseKeyUp may be called a second time, depending on where the
mouse is when the button is
// released, but since `this.pressed` will no longer be true, the
second call will be ignored.
- this.onPointerUp( e );
+ this.onMouseKeyUp( e );
};
/**
- * Handle mouse up events.
+ * Handle mouse up and key up events.
*
- * @param {jQuery.Event} e Mouse up event
+ * @param {jQuery.Event} e Mouse up or key up event
*/
-OO.ui.ToolGroup.prototype.onPointerUp = function ( e ) {
+OO.ui.ToolGroup.prototype.onMouseKeyUp = function ( e ) {
var tool = this.getTargetTool( e );
- if ( !this.isDisabled() && e.which === 1 && this.pressed &&
this.pressed === tool ) {
+ if (
+ !this.isDisabled() && this.pressed && this.pressed === tool &&
+ ( e.which === 1 || e.which === OO.ui.Keys.SPACE || e.which ===
OO.ui.Keys.ENTER )
+ ) {
this.pressed.onSelect();
+ this.pressed = null;
+ return false;
}
this.pressed = null;
- return false;
};
/**
- * Handle mouse over events.
+ * Handle mouse over and focus events.
*
- * @param {jQuery.Event} e Mouse over event
+ * @param {jQuery.Event} e Mouse over or focus event
*/
-OO.ui.ToolGroup.prototype.onMouseOver = function ( e ) {
+OO.ui.ToolGroup.prototype.onMouseOverFocus = function ( e ) {
var tool = this.getTargetTool( e );
if ( this.pressed && this.pressed === tool ) {
@@ -194,11 +205,11 @@
};
/**
- * Handle mouse out events.
+ * Handle mouse out and blur events.
*
- * @param {jQuery.Event} e Mouse out event
+ * @param {jQuery.Event} e Mouse out or blur event
*/
-OO.ui.ToolGroup.prototype.onMouseOut = function ( e ) {
+OO.ui.ToolGroup.prototype.onMouseOutBlur = function ( e ) {
var tool = this.getTargetTool( e );
if ( this.pressed && this.pressed === tool ) {
diff --git a/src/Toolbar.js b/src/Toolbar.js
index 7bc2cad..19f09c2 100644
--- a/src/Toolbar.js
+++ b/src/Toolbar.js
@@ -170,7 +170,7 @@
// Events
this.$element
.add( this.$bar ).add( this.$group ).add( this.$actions )
- .on( 'mousedown', this.onPointerDown.bind( this ) );
+ .on( 'mousedown keydown', this.onPointerDown.bind( this ) );
// Initialization
this.$group.addClass( 'oo-ui-toolbar-tools' );
diff --git a/src/toolgroups/ListToolGroup.js b/src/toolgroups/ListToolGroup.js
index 426be5a..91227ba 100644
--- a/src/toolgroups/ListToolGroup.js
+++ b/src/toolgroups/ListToolGroup.js
@@ -108,16 +108,18 @@
/**
* @inheritdoc
*/
-OO.ui.ListToolGroup.prototype.onPointerUp = function ( e ) {
- var ret = OO.ui.ListToolGroup.super.prototype.onPointerUp.call( this, e
);
-
+OO.ui.ListToolGroup.prototype.onMouseKeyUp = function ( e ) {
// Do not close the popup when the user wants to show more/fewer tools
- if ( $( e.target ).closest( '.oo-ui-tool-name-more-fewer' ).length ) {
- // Prevent the popup list from being hidden
- this.setActive( true );
+ if (
+ $( e.target ).closest( '.oo-ui-tool-name-more-fewer' ).length &&
+ ( e.which === 1 || e.which === OO.ui.Keys.SPACE || e.which ===
OO.ui.Keys.ENTER )
+ ) {
+ // HACK: Prevent the popup list from being hidden. Skip the
PopupToolGroup implementation (which
+ // hides the popup list when a tool is selected) and call
ToolGroup's implementation directly.
+ return
OO.ui.ListToolGroup.super.super.prototype.onMouseKeyUp.call( this, e );
+ } else {
+ return OO.ui.ListToolGroup.super.prototype.onMouseKeyUp.call(
this, e );
}
-
- return ret;
};
OO.ui.ListToolGroup.prototype.updateCollapsibleState = function () {
diff --git a/src/toolgroups/PopupToolGroup.js b/src/toolgroups/PopupToolGroup.js
index 437c3d0..d9f4954 100644
--- a/src/toolgroups/PopupToolGroup.js
+++ b/src/toolgroups/PopupToolGroup.js
@@ -9,6 +9,7 @@
* @mixins OO.ui.LabelElement
* @mixins OO.ui.TitledElement
* @mixins OO.ui.ClippableElement
+ * @mixins OO.ui.TabIndexedElement
*
* @constructor
* @param {OO.ui.Toolbar} toolbar
@@ -28,23 +29,26 @@
// Parent constructor
OO.ui.PopupToolGroup.super.call( this, toolbar, config );
- // Mixin constructors
- OO.ui.IconElement.call( this, config );
- OO.ui.IndicatorElement.call( this, config );
- OO.ui.LabelElement.call( this, config );
- OO.ui.TitledElement.call( this, config );
- OO.ui.ClippableElement.call( this, $.extend( {}, config, { $clippable:
this.$group } ) );
-
// Properties
this.active = false;
this.dragging = false;
this.onBlurHandler = this.onBlur.bind( this );
this.$handle = $( '<span>' );
+ // Mixin constructors
+ OO.ui.IconElement.call( this, config );
+ OO.ui.IndicatorElement.call( this, config );
+ OO.ui.LabelElement.call( this, config );
+ OO.ui.TitledElement.call( this, config );
+ OO.ui.ClippableElement.call( this, $.extend( {}, config, { $clippable:
this.$group } ) );
+ OO.ui.TabIndexedElement.call( this, $.extend( {}, config, {
$tabIndexed: this.$handle } ) );
+
// Events
this.$handle.on( {
- mousedown: this.onHandlePointerDown.bind( this ),
- mouseup: this.onHandlePointerUp.bind( this )
+ keydown: this.onHandleMouseKeyDown.bind( this ),
+ keyup: this.onHandleMouseKeyUp.bind( this ),
+ mousedown: this.onHandleMouseKeyDown.bind( this ),
+ mouseup: this.onHandleMouseKeyUp.bind( this )
} );
// Initialization
@@ -74,6 +78,7 @@
OO.mixinClass( OO.ui.PopupToolGroup, OO.ui.LabelElement );
OO.mixinClass( OO.ui.PopupToolGroup, OO.ui.TitledElement );
OO.mixinClass( OO.ui.PopupToolGroup, OO.ui.ClippableElement );
+OO.mixinClass( OO.ui.PopupToolGroup, OO.ui.TabIndexedElement );
/* Static Properties */
@@ -94,9 +99,9 @@
/**
* Handle focus being lost.
*
- * The event is actually generated from a mouseup, so it is not a normal blur
event object.
+ * The event is actually generated from a mouseup/keyup, so it is not a normal
blur event object.
*
- * @param {jQuery.Event} e Mouse up event
+ * @param {jQuery.Event} e Mouse up or key up event
*/
OO.ui.PopupToolGroup.prototype.onBlur = function ( e ) {
// Only deactivate when clicking outside the dropdown element
@@ -108,33 +113,44 @@
/**
* @inheritdoc
*/
-OO.ui.PopupToolGroup.prototype.onPointerUp = function ( e ) {
+OO.ui.PopupToolGroup.prototype.onMouseKeyUp = function ( e ) {
// Only close toolgroup when a tool was actually selected
- if ( !this.isDisabled() && e.which === 1 && this.pressed &&
this.pressed === this.getTargetTool( e ) ) {
+ if (
+ !this.isDisabled() && this.pressed && this.pressed ===
this.getTargetTool( e ) &&
+ ( e.which === 1 || e.which === OO.ui.Keys.SPACE || e.which ===
OO.ui.Keys.ENTER )
+ ) {
this.setActive( false );
}
- return OO.ui.PopupToolGroup.super.prototype.onPointerUp.call( this, e );
+ return OO.ui.PopupToolGroup.super.prototype.onMouseKeyUp.call( this, e
);
};
/**
- * Handle mouse up events.
+ * Handle mouse up and key up events.
*
- * @param {jQuery.Event} e Mouse up event
+ * @param {jQuery.Event} e Mouse up or key up event
*/
-OO.ui.PopupToolGroup.prototype.onHandlePointerUp = function () {
- return false;
-};
-
-/**
- * Handle mouse down events.
- *
- * @param {jQuery.Event} e Mouse down event
- */
-OO.ui.PopupToolGroup.prototype.onHandlePointerDown = function ( e ) {
- if ( !this.isDisabled() && e.which === 1 ) {
- this.setActive( !this.active );
+OO.ui.PopupToolGroup.prototype.onHandleMouseKeyUp = function ( e ) {
+ if (
+ !this.isDisabled() &&
+ ( e.which === 1 || e.which === OO.ui.Keys.SPACE || e.which ===
OO.ui.Keys.ENTER )
+ ) {
+ return false;
}
- return false;
+};
+
+/**
+ * Handle mouse down and key down events.
+ *
+ * @param {jQuery.Event} e Mouse down or key down event
+ */
+OO.ui.PopupToolGroup.prototype.onHandleMouseKeyDown = function ( e ) {
+ if (
+ !this.isDisabled() &&
+ ( e.which === 1 || e.which === OO.ui.Keys.SPACE || e.which ===
OO.ui.Keys.ENTER )
+ ) {
+ this.setActive( !this.active );
+ return false;
+ }
};
/**
@@ -148,6 +164,7 @@
this.active = value;
if ( value ) {
this.getElementDocument().addEventListener( 'mouseup',
this.onBlurHandler, true );
+ this.getElementDocument().addEventListener( 'keyup',
this.onBlurHandler, true );
// Try anchoring the popup to the left first
this.$element.addClass( 'oo-ui-popupToolGroup-active
oo-ui-popupToolGroup-left' );
@@ -162,6 +179,7 @@
}
} else {
this.getElementDocument().removeEventListener(
'mouseup', this.onBlurHandler, true );
+ this.getElementDocument().removeEventListener( 'keyup',
this.onBlurHandler, true );
this.$element.removeClass(
'oo-ui-popupToolGroup-active
oo-ui-popupToolGroup-left oo-ui-popupToolGroup-right'
);
--
To view, visit https://gerrit.wikimedia.org/r/206643
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4ca8c77bdcea8ea8472d013af28a41361bf69065
Gerrit-PatchSet: 1
Gerrit-Project: oojs/ui
Gerrit-Branch: master
Gerrit-Owner: Bartosz Dziewoński <matma....@gmail.com>
Gerrit-Reviewer: Esanders <esand...@wikimedia.org>
Gerrit-Reviewer: TheDJ <hartman.w...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
