Skip to content
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.

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

Better user interface for screen readers and keyboard navigation #2946

Merged
merged 1 commit into from
Feb 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions src/components/structures/BottomLeftMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ var React = require('react');
var ReactDOM = require('react-dom');
var sdk = require('matrix-react-sdk')
var dis = require('matrix-react-sdk/lib/dispatcher');
var AccessibleButton = require('matrix-react-sdk/lib/components/views/elements/AccessibleButton');

module.exports = React.createClass({
displayName: 'BottomLeftMenu',
Expand Down Expand Up @@ -101,22 +102,22 @@ module.exports = React.createClass({
return (
<div className="mx_BottomLeftMenu">
<div className="mx_BottomLeftMenu_options">
<div className="mx_BottomLeftMenu_people" onClick={ this.onPeopleClick } onMouseEnter={ this.onPeopleMouseEnter } onMouseLeave={ this.onPeopleMouseLeave } >
<AccessibleButton className="mx_BottomLeftMenu_people" onClick={ this.onPeopleClick } onMouseEnter={ this.onPeopleMouseEnter } onMouseLeave={ this.onPeopleMouseLeave } >
<TintableSvg src="img/icons-people.svg" width="25" height="25" />
{ this.getLabel("Start chat", this.state.peopleHover) }
</div>
<div className="mx_BottomLeftMenu_directory" onClick={ this.onDirectoryClick } onMouseEnter={ this.onDirectoryMouseEnter } onMouseLeave={ this.onDirectoryMouseLeave } >
</AccessibleButton>
<AccessibleButton className="mx_BottomLeftMenu_directory" onClick={ this.onDirectoryClick } onMouseEnter={ this.onDirectoryMouseEnter } onMouseLeave={ this.onDirectoryMouseLeave } >
<TintableSvg src="img/icons-directory.svg" width="25" height="25"/>
{ this.getLabel("Room directory", this.state.directoryHover) }
</div>
<div className="mx_BottomLeftMenu_createRoom" onClick={ this.onRoomsClick } onMouseEnter={ this.onRoomsMouseEnter } onMouseLeave={ this.onRoomsMouseLeave } >
</AccessibleButton>
<AccessibleButton className="mx_BottomLeftMenu_createRoom" onClick={ this.onRoomsClick } onMouseEnter={ this.onRoomsMouseEnter } onMouseLeave={ this.onRoomsMouseLeave } >
<TintableSvg src="img/icons-create-room.svg" width="25" height="25" />
{ this.getLabel("Create new room", this.state.roomsHover) }
</div>
<div className="mx_BottomLeftMenu_settings" onClick={ this.onSettingsClick } onMouseEnter={ this.onSettingsMouseEnter } onMouseLeave={ this.onSettingsMouseLeave } >
</AccessibleButton>
<AccessibleButton className="mx_BottomLeftMenu_settings" onClick={ this.onSettingsClick } onMouseEnter={ this.onSettingsMouseEnter } onMouseLeave={ this.onSettingsMouseLeave } >
<TintableSvg src="img/icons-settings.svg" width="25" height="25" />
{ this.getLabel("Settings", this.state.settingsHover) }
</div>
</AccessibleButton>
</div>
</div>
);
Expand Down
17 changes: 12 additions & 5 deletions src/components/structures/RightPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ var dis = require('matrix-react-sdk/lib/dispatcher');
var MatrixClientPeg = require("matrix-react-sdk/lib/MatrixClientPeg");
var rate_limited_func = require('matrix-react-sdk/lib/ratelimitedfunc');
var Modal = require('matrix-react-sdk/lib/Modal');
var AccessibleButton = require('matrix-react-sdk/lib/components/views/elements/AccessibleButton');

module.exports = React.createClass({
displayName: 'RightPanel',
Expand Down Expand Up @@ -207,33 +208,39 @@ module.exports = React.createClass({

if (user_is_in_room) {
inviteGroup =
<div className="mx_RightPanel_invite" onClick={ this.onInviteButtonClick } >
<AccessibleButton className="mx_RightPanel_invite" onClick={ this.onInviteButtonClick } >
<div className="mx_RightPanel_icon" >
<TintableSvg src="img/icon-invite-people.svg" width="35" height="35" />
</div>
<div className="mx_RightPanel_message">Invite to this room</div>
</div>;
</AccessibleButton>;
}

}

if (this.props.roomId) {
buttonGroup =
<div className="mx_RightPanel_headerButtonGroup">
<div className="mx_RightPanel_headerButton" title="Members" onClick={ this.onMemberListButtonClick }>
<div className="mx_RightPanel_headerButton">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are the divs still needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't want to touch it just in case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like it'd be fine if the class were to be moved to the button element.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got rid of the divs.

<AccessibleButton title="Members" onClick={ this.onMemberListButtonClick }>
<div className="mx_RightPanel_headerButton_badge">{ membersBadge ? membersBadge : <span>&nbsp;</span>}</div>
<TintableSvg src="img/icons-people.svg" width="25" height="25"/>
{ membersHighlight }
</AccessibleButton>
</div>
<div className="mx_RightPanel_headerButton mx_RightPanel_filebutton" title="Files" onClick={ this.onFileListButtonClick }>
<div className="mx_RightPanel_headerButton mx_RightPanel_filebutton">
<AccessibleButton title="Files" onClick={ this.onFileListButtonClick }>
<div className="mx_RightPanel_headerButton_badge">&nbsp;</div>
<TintableSvg src="img/icons-files.svg" width="25" height="25"/>
{ filesHighlight }
</AccessibleButton>
</div>
<div className="mx_RightPanel_headerButton mx_RightPanel_notificationbutton" title="Notifications" onClick={ this.onNotificationListButtonClick }>
<div className="mx_RightPanel_headerButton mx_RightPanel_notificationbutton">
<AccessibleButton title="Notifications" onClick={ this.onNotificationListButtonClick }>
<div className="mx_RightPanel_headerButton_badge">&nbsp;</div>
<TintableSvg src="img/icons-notifications.svg" width="25" height="25"/>
{ notificationsHighlight }
</AccessibleButton>
</div>
</div>;
}
Expand Down
13 changes: 8 additions & 5 deletions src/components/structures/RoomSubList.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ var Unread = require('matrix-react-sdk/lib/Unread');
var MatrixClientPeg = require('matrix-react-sdk/lib/MatrixClientPeg');
var RoomNotifs = require('matrix-react-sdk/lib/RoomNotifs');
var FormattingUtils = require('matrix-react-sdk/lib/utils/FormattingUtils');
var AccessibleButton = require('matrix-react-sdk/lib/components/views/elements/AccessibleButton');

// turn this on for drop & drag console debugging galore
var debug = false;
Expand Down Expand Up @@ -417,15 +418,17 @@ var RoomSubList = React.createClass({
}
}

var tabindex = this.props.searchFilter === "" ? "0" : "-1";

return (
<div className="mx_RoomSubList_labelContainer" title={ title } ref="header">
<div onClick={ this.onClick } className="mx_RoomSubList_label">
<AccessibleButton onClick={ this.onClick } className="mx_RoomSubList_label" tabIndex={tabindex}>
{ this.props.collapsed ? '' : this.props.label }
<div className="mx_RoomSubList_roomCount">{ roomCount }</div>
<div className={chevronClasses}></div>
{ badge }
{ incomingCall }
</div>
</AccessibleButton>
</div>
);
},
Expand All @@ -447,11 +450,11 @@ var RoomSubList = React.createClass({
});

return (
<div className="mx_RoomSubList_ellipsis" onClick={this._showFullMemberList}>
<AccessibleButton className="mx_RoomSubList_ellipsis" onClick={this._showFullMemberList}>
<div className="mx_RoomSubList_line"></div>
<div className="mx_RoomSubList_more">more</div>
<div className={ badgeClasses }>{ content }</div>
</div>
<div className={ badgeClasses }>{ content }</div>
</AccessibleButton>
);
},

Expand Down
43 changes: 35 additions & 8 deletions src/components/structures/SearchBox.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ var React = require('react');
var sdk = require('matrix-react-sdk')
var dis = require('matrix-react-sdk/lib/dispatcher');
var rate_limited_func = require('matrix-react-sdk/lib/ratelimitedfunc');
var AccessibleButton = require('matrix-react-sdk/lib/components/views/elements/AccessibleButton');

module.exports = React.createClass({
displayName: 'SearchBox',
Expand All @@ -35,6 +36,25 @@ module.exports = React.createClass({
};
},

componentDidMount: function() {
this.dispatcherRef = dis.register(this.onAction);
},

componentWillUnmount: function() {
dis.unregister(this.dispatcherRef);
},

onAction: function(payload) {
switch (payload.action) {
// Clear up the text field when a room is selected.
case 'view_room':
if (this.refs.search) {
this._clearSearch();
}
break;
}
},

onChange: function() {
if (!this.refs.search) return;
this.setState({ searchTerm: this.refs.search.value });
Expand All @@ -61,35 +81,42 @@ module.exports = React.createClass({
}
},

_clearSearch: function() {
this.refs.search.value = "";
this.onChange();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should happen automatically?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're kinda right, for reasons that are ... non-obvious. In case you're interested, and for my own reference:

Essentially the onChange callback is a React fabrication. Native-DOM <input> elements have an onchange attribute, but it is only called when the element loses focus.

React actually listens for the input event, and calls the onChange callback when it sees one. Apparently the DOM does not fire an input event when the value is changed from js - which is probably lucky otherwise React would have to jump through hoops to avoid getting stuck in a loop.

What we ought to do here is stick with the react side, and just make the change through react:

this.setState({ searchTerm: "" });

That will trigger a re-render, which will update the DOM.

See also: https://facebook.github.io/react/docs/forms.html#controlled-components.

},

render: function() {
var TintableSvg = sdk.getComponent('elements.TintableSvg');

var collapseTabIndex = this.refs.search && this.refs.search.value !== "" ? "-1" : "0";

var toggleCollapse;
if (this.props.collapsed) {
toggleCollapse =
<div className="mx_SearchBox_maximise" onClick={ this.onToggleCollapse.bind(this, true) }>
<AccessibleButton className="mx_SearchBox_maximise" tabIndex={collapseTabIndex} onClick={ this.onToggleCollapse.bind(this, true) }>
<TintableSvg src="img/maximise.svg" width="10" height="16" alt="&lt;"/>
</div>
</AccessibleButton>
}
else {
toggleCollapse =
<div className="mx_SearchBox_minimise" onClick={ this.onToggleCollapse.bind(this, false) }>
<AccessibleButton className="mx_SearchBox_minimise" tabIndex={collapseTabIndex} onClick={ this.onToggleCollapse.bind(this, false) }>
<TintableSvg src="img/minimise.svg" width="10" height="16" alt="&lt;"/>
</div>
</AccessibleButton>
}

var searchControls;
if (!this.props.collapsed) {
searchControls = [
this.state.searchTerm.length > 0 ?
<div key="button"
className="mx_SearchBox_closeButton"
onClick={ ()=>{ this.refs.search.value = ""; this.onChange(); } }>
<AccessibleButton key="button"
className="mx_SearchBox_closeButton"
onClick={ ()=>{ this._clearSearch(); } }>
<TintableSvg
className="mx_SearchBox_searchButton"
src="img/icons-close.svg" width="24" height="24"
/>
</div>
</AccessibleButton>
:
<TintableSvg
key="button"
Expand Down
3 changes: 2 additions & 1 deletion src/components/views/elements/ImageView.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ var MatrixClientPeg = require('matrix-react-sdk/lib/MatrixClientPeg');

var DateUtils = require('matrix-react-sdk/lib/DateUtils');
var filesize = require('filesize');
var AccessibleButton = require('matrix-react-sdk/lib/components/views/elements/AccessibleButton');

module.exports = React.createClass({
displayName: 'ImageView',
Expand Down Expand Up @@ -162,7 +163,7 @@ module.exports = React.createClass({
<img src={this.props.src} style={style}/>
<div className="mx_ImageView_labelWrapper">
<div className="mx_ImageView_label">
<img className="mx_ImageView_cancel" src="img/cancel-white.svg" width="18" height="18" alt="Close" onClick={ this.props.onFinished }/>
<AccessibleButton className="mx_ImageView_cancel" onClick={ this.props.onFinished }><img src="img/cancel-white.svg" width="18" height="18" alt="Close"/></AccessibleButton>
<div className="mx_ImageView_shim">
</div>
<div className="mx_ImageView_name">
Expand Down
3 changes: 2 additions & 1 deletion src/components/views/globals/MatrixToolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ limitations under the License.
var React = require('react');
var Notifier = require("matrix-react-sdk/lib/Notifier");
var sdk = require('matrix-react-sdk')
var AccessibleButton = require('matrix-react-sdk/lib/components/views/elements/AccessibleButton');

module.exports = React.createClass({
displayName: 'MatrixToolbar',
Expand All @@ -38,7 +39,7 @@ module.exports = React.createClass({
<div className="mx_MatrixToolbar_content">
You are not receiving desktop notifications. <a className="mx_MatrixToolbar_link" onClick={ this.onClick }>Enable them now</a>
</div>
<div className="mx_MatrixToolbar_close"><img src="img/cancel.svg" width="18" height="18" onClick={ this.hideToolbar } /></div>
<AccessibleButton className="mx_MatrixToolbar_close" onClick={ this.hideToolbar } ><img src="img/cancel.svg" width="18" height="18" /></AccessibleButton>
</div>
);
}
Expand Down
11 changes: 6 additions & 5 deletions src/components/views/rooms/SearchBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ var React = require('react');
var MatrixClientPeg = require('matrix-react-sdk/lib/MatrixClientPeg');
var sdk = require('matrix-react-sdk');
var classNames = require('classnames');
var AccessibleButton = require('matrix-react-sdk/lib/components/views/elements/AccessibleButton');

module.exports = React.createClass({
displayName: 'SearchBar',
Expand Down Expand Up @@ -57,12 +58,12 @@ module.exports = React.createClass({
var allRoomsClasses = classNames({ mx_SearchBar_button : true, mx_SearchBar_unselected : this.state.scope !== 'All' });

return (
<div className="mx_SearchBar">
<div className="mx_SearchBar">
<input ref="search_term" className="mx_SearchBar_input" type="text" autoFocus={true} placeholder="Search..." onKeyDown={this.onSearchChange}/>
<div className={ searchButtonClasses } onClick={this.onSearch}><img src="img/search-button.svg" width="37" height="37" alt="Search"/></div>
<div className={ thisRoomClasses } onClick={this.onThisRoomClick}>This Room</div>
<div className={ allRoomsClasses } onClick={this.onAllRoomsClick}>All Rooms</div>
<img className="mx_SearchBar_cancel" src="img/cancel.svg" width="18" height="18" onClick={this.props.onCancelClick} />
<AccessibleButton className={ searchButtonClasses } onClick={this.onSearch}><img src="img/search-button.svg" width="37" height="37" alt="Search"/></AccessibleButton>
<AccessibleButton className={ thisRoomClasses } onClick={this.onThisRoomClick}>This Room</AccessibleButton>
<AccessibleButton className={ allRoomsClasses } onClick={this.onAllRoomsClick}>All Rooms</AccessibleButton>
<AccessibleButton className="mx_SearchBar_cancel" onClick={this.props.onCancelClick}><img src="img/cancel.svg" width="18" height="18" /></AccessibleButton>
</div>
);
}
Expand Down