Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix UX issues with bug report dialog #1863

Merged
merged 16 commits into from
Apr 30, 2018
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
1 change: 1 addition & 0 deletions res/css/_common.scss
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ textarea {
.mx_Dialog button.danger, .mx_Dialog input[type="submit"].danger {
background-color: $warning-color;
border: solid 1px $warning-color;
color: $accent-fg-color;
Copy link
Contributor

Choose a reason for hiding this comment

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

From a code perspective, fine. From a UI perspective though, this text should very much be white.

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently the text on all the red buttons is black in the dark theme

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh. Sorry, I should've looked at what the colours actually are.

}

.mx_Dialog button:disabled, .mx_Dialog input[type="submit"]:disabled {
Expand Down
4 changes: 2 additions & 2 deletions src/components/structures/UserSettings.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
Copyright 2015, 2016 OpenMarket Ltd
Copyright 2017 Vector Creations Ltd
Copyright 2017 New Vector Ltd
Copyright 2017, 2018 New Vector Ltd

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -804,7 +804,7 @@ module.exports = React.createClass({
"other users. They do not contain messages.",
)
}</p>
<button className="mx_UserSettings_button danger"
<button className="mx_UserSettings_button"
onClick={this._onBugReportClicked}>{ _t('Submit debug logs') }
</button>
</div>
Expand Down
20 changes: 17 additions & 3 deletions src/components/views/dialogs/BaseDialog.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
Copyright 2017 Vector Creations Ltd
Copyright 2018 New Vector Ltd

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -41,6 +42,13 @@ export default React.createClass({
// cancelled (BaseDialog itself only calls this with false).
onFinished: PropTypes.func.isRequired,

// Whether the dialog should have a 'close' button that will
// cause the dialog to be cancelled. This should only be set
// to false if there is nothing the app can sensibly do if the
// dialog is cancelled, eg. "We can't restore your session and
// the app cannot work". Default: true.
hasCancel: PropTypes.bool,

// called when a key is pressed
onKeyDown: PropTypes.func,

Expand All @@ -59,6 +67,12 @@ export default React.createClass({
contentId: React.PropTypes.string,
},

getDefaultProps: function() {
return {
hasCancel: true,
};
},

childContextTypes: {
matrixClient: PropTypes.instanceOf(MatrixClient),
},
Expand All @@ -77,7 +91,7 @@ export default React.createClass({
if (this.props.onKeyDown) {
this.props.onKeyDown(e);
}
if (e.keyCode === KeyCode.ESCAPE) {
if (this.props.hasCancel && e.keyCode === KeyCode.ESCAPE) {
e.stopPropagation();
e.preventDefault();
this.props.onFinished(false);
Expand All @@ -104,11 +118,11 @@ export default React.createClass({
// AT users can skip its presentation.
aria-describedby={this.props.contentId}
>
<AccessibleButton onClick={this._onCancelClick}
{ this.props.hasCancel ? <AccessibleButton onClick={this._onCancelClick}
className="mx_Dialog_cancelButton"
>
<TintableSvg src="img/icons-close-button.svg" width="35" height="35" />
</AccessibleButton>
</AccessibleButton> : null }
<div className={'mx_Dialog_title ' + this.props.titleClass} id='mx_BaseDialog_title'>
{ this.props.title }
</div>
Expand Down
42 changes: 16 additions & 26 deletions src/components/views/dialogs/BugReportDialog.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
Copyright 2017 OpenMarket Ltd
Copyright 2018 New Vector Ltd

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -105,6 +106,8 @@ export default class BugReportDialog extends React.Component {

render() {
const Loader = sdk.getComponent("elements.Spinner");
const BaseDialog = sdk.getComponent('views.dialogs.BaseDialog');
const DialogButtons = sdk.getComponent('views.elements.DialogButtons');

let error = null;
if (this.state.err) {
Expand All @@ -113,13 +116,6 @@ export default class BugReportDialog extends React.Component {
</div>;
}

let cancelButton = null;
if (!this.state.busy) {
cancelButton = <button onClick={this._onCancel}>
{ _t("Cancel") }
</button>;
}

let progress = null;
if (this.state.busy) {
progress = (
Expand All @@ -131,11 +127,11 @@ export default class BugReportDialog extends React.Component {
}

return (
<div className="mx_BugReportDialog">
<div className="mx_Dialog_title">
{ _t("Submit debug logs") }
</div>
<div className="mx_Dialog_content">
<BaseDialog className="mx_BugReportDialog" onFinished={this._onCancel}
title={_t('Submit debug logs')}
contentId='mx_Dialog_content'
>
<div className="mx_Dialog_content" id='mx_Dialog_content'>
<p>
{ _t(
"Debug logs contain application usage data including your " +
Expand All @@ -146,7 +142,7 @@ export default class BugReportDialog extends React.Component {
</p>
<p>
{ _t(
"<a>Click here</a> to create a GitHub issue.",
"Riot bugs are tracked on GitHub: <a>create a GitHub issue</a>.",
{},
{
a: (sub) => <a
Expand Down Expand Up @@ -191,19 +187,13 @@ export default class BugReportDialog extends React.Component {
{progress}
{error}
</div>
<div className="mx_Dialog_buttons">
<button
className="mx_Dialog_primary danger"
onClick={this._onSubmit}
autoFocus={true}
disabled={this.state.busy}
>
{ _t("Send logs") }
</button>

{cancelButton}
</div>
</div>
<DialogButtons primaryButton={_t("Send logs")}
onPrimaryButtonClick={this._onSubmit}
focus={true}
onCancel={this._onCancel}
disabled={this.state.busy}
/>
</BaseDialog>
);
}
}
Expand Down
86 changes: 51 additions & 35 deletions src/components/views/dialogs/SessionRestoreErrorDialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,63 +31,79 @@ export default React.createClass({
onFinished: PropTypes.func.isRequired,
},

componentDidMount: function() {
if (this.refs.bugreportLink) {
this.refs.bugreportLink.focus();
}
},

_sendBugReport: function() {
const BugReportDialog = sdk.getComponent("dialogs.BugReportDialog");
Modal.createTrackedDialog('Session Restore Error', 'Send Bug Report Dialog', BugReportDialog, {});
},

_onContinueClick: function() {
this.props.onFinished(true);
_onClearStorageClick: function() {
const QuestionDialog = sdk.getComponent("dialogs.QuestionDialog");
Modal.createTrackedDialog('Session Restore Confirm Logout', '', QuestionDialog, {
title: _t("Sign out"),
description:
<div>{ _t("Log out and remove encryption keys?") }</div>,
button: _t("Sign out"),
danger: true,
onFinished: this.props.onFinished,
});
},

_onCancelClick: function() {
this.props.onFinished(false);
_onRefreshClick: function() {
// Is this likely to help? Probably not, but giving only one button
// that clears your storage seems awful.
window.location.reload(true);
},

render: function() {
const BaseDialog = sdk.getComponent('views.dialogs.BaseDialog');
const DialogButtons = sdk.getComponent('views.elements.DialogButtons');
let bugreport;

const clearStorageButton = (
<button onClick={this._onClearStorageClick} className="danger">
{ _t("Clear Storage and Sign Out") }
</button>
);

let dialogButtons;
if (SdkConfig.get().bug_report_endpoint_url) {
bugreport = (
<p>
{ _t(
"Otherwise, <a>click here</a> to send a bug report.",
{},
{ 'a': (sub) => <a ref="bugreportLink" onClick={this._sendBugReport}
key="bugreport" href='#'>{ sub }</a> },
) }
</p>
);
dialogButtons = <DialogButtons primaryButton={_t("Send Logs")}
onPrimaryButtonClick={this._sendBugReport}
focus={true}
hasCancel={false}
>
{ clearStorageButton }
</DialogButtons>;
} else {
dialogButtons = <DialogButtons primaryButton={_t("Refresh")}
onPrimaryButtonClick={this._onRefreshClick}
focus={true}
hasCancel={false}
>
{ clearStorageButton }
</DialogButtons>;
}
const shouldFocusContinueButton =!(bugreport==true);

return (
<BaseDialog className="mx_ErrorDialog" onFinished={this.props.onFinished}
title={_t('Unable to restore session')}
title={_t('Unable to restore session')}
contentId='mx_Dialog_content'
hasCancel={false}
>
<div className="mx_Dialog_content" id='mx_Dialog_content'>
<p>{ _t("We encountered an error trying to restore your previous session. If " +
"you continue, you will need to log in again, and encrypted chat " +
"history will be unreadable.") }</p>

<p>{ _t("If you have previously used a more recent version of Riot, your session " +
"may be incompatible with this version. Close this window and return " +
"to the more recent version.") }</p>

{ bugreport }
<p>{ _t("We encountered an error trying to restore your previous session.") }</p>

<p>{ _t(
"If you have previously used a more recent version of Riot, your session " +
"may be incompatible with this version. Close this window and return " +
"to the more recent version.",
) }</p>

<p>{ _t(
"Clearing your browser's storage may fix the problem, but will sign you " +
"out and cause any encrypted chat history to become unreadable.",
) }</p>
</div>
<DialogButtons primaryButton={_t("Continue anyway")}
onPrimaryButtonClick={this._onContinueClick} focus={shouldFocusContinueButton}
onCancel={this._onCancelClick} />
{ dialogButtons }
</BaseDialog>
);
},
Expand Down
28 changes: 22 additions & 6 deletions src/components/views/elements/DialogButtons.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
Copyright 2017 Aidan Gauland
Copyright 2018 New Vector Ltd.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -14,8 +15,6 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

"use strict";

import React from "react";
import PropTypes from "prop-types";
import { _t } from '../../../languageHandler';
Expand All @@ -33,10 +32,22 @@ module.exports = React.createClass({
// onClick handler for the primary button.
onPrimaryButtonClick: PropTypes.func.isRequired,

// should there be a cancel button? default: true
hasCancel: PropTypes.bool,

// onClick handler for the cancel button.
onCancel: PropTypes.func.isRequired,
onCancel: PropTypes.func,

focus: PropTypes.bool,

disabled: PropTypes.bool,
},

getDefaultProps: function() {
return {
hasCancel: true,
disabled: false,
};
},

_onCancelClick: function() {
Expand All @@ -48,18 +59,23 @@ module.exports = React.createClass({
if (this.props.primaryButtonClass) {
primaryButtonClassName += " " + this.props.primaryButtonClass;
}
let cancelButton;
if (this.props.hasCancel) {
cancelButton = <button onClick={this._onCancelClick} disabled={this.props.disabled}>
{ _t("Cancel") }
</button>;
}
return (
<div className="mx_Dialog_buttons">
<button className={primaryButtonClassName}
onClick={this.props.onPrimaryButtonClick}
autoFocus={this.props.focus}
disabled={this.props.disabled}
>
{ this.props.primaryButton }
</button>
{ this.props.children }
<button onClick={this._onCancelClick}>
{ _t("Cancel") }
</button>
{ cancelButton }
</div>
);
},
Expand Down
15 changes: 9 additions & 6 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@
"You need to be logged in.": "You need to be logged in.",
"You need to be able to invite users to do that.": "You need to be able to invite users to do that.",
"Unable to create widget.": "Unable to create widget.",
"Popout widget": "Popout widget",
"Missing roomId.": "Missing roomId.",
"Failed to send request.": "Failed to send request.",
"This room is not recognised.": "This room is not recognised.",
Expand Down Expand Up @@ -655,6 +654,7 @@
"Delete widget": "Delete widget",
"Revoke widget access": "Revoke widget access",
"Minimize apps": "Minimize apps",
"Popout widget": "Popout widget",
"Picture": "Picture",
"Edit": "Edit",
"Create new room": "Create new room",
Expand Down Expand Up @@ -745,7 +745,7 @@
"Failed to send logs: ": "Failed to send logs: ",
"Submit debug logs": "Submit debug logs",
"Debug logs contain application usage data including your username, the IDs or aliases of the rooms or groups you have visited and the usernames of other users. They do not contain messages.": "Debug logs contain application usage data including your username, the IDs or aliases of the rooms or groups you have visited and the usernames of other users. They do not contain messages.",
"<a>Click here</a> to create a GitHub issue.": "<a>Click here</a> to create a GitHub issue.",
"Riot bugs are tracked on GitHub: <a>create a GitHub issue</a>.": "Riot bugs are tracked on GitHub: <a>create a GitHub issue</a>.",
"GitHub issue link:": "GitHub issue link:",
"Notes:": "Notes:",
"Send logs": "Send logs",
Expand Down Expand Up @@ -807,11 +807,15 @@
"Ignore request": "Ignore request",
"Loading device info...": "Loading device info...",
"Encryption key request": "Encryption key request",
"Otherwise, <a>click here</a> to send a bug report.": "Otherwise, <a>click here</a> to send a bug report.",
"Sign out": "Sign out",
"Log out and remove encryption keys?": "Log out and remove encryption keys?",
"Send Logs": "Send Logs",
"Clear Storage and Sign Out": "Clear Storage and Sign Out",
"Refresh": "Refresh",
"Unable to restore session": "Unable to restore session",
"We encountered an error trying to restore your previous session. If you continue, you will need to log in again, and encrypted chat history will be unreadable.": "We encountered an error trying to restore your previous session. If you continue, you will need to log in again, and encrypted chat history will be unreadable.",
"We encountered an error trying to restore your previous session.": "We encountered an error trying to restore your previous session.",
"If you have previously used a more recent version of Riot, your session may be incompatible with this version. Close this window and return to the more recent version.": "If you have previously used a more recent version of Riot, your session may be incompatible with this version. Close this window and return to the more recent version.",
"Continue anyway": "Continue anyway",
"Clearing your browser's storage may fix the problem, but will sign you out and cause any encrypted chat history to become unreadable.": "Clearing your browser's storage may fix the problem, but will sign you out and cause any encrypted chat history to become unreadable.",
"Invalid Email Address": "Invalid Email Address",
"This doesn't appear to be a valid email address": "This doesn't appear to be a valid email address",
"Verification Pending": "Verification Pending",
Expand Down Expand Up @@ -1015,7 +1019,6 @@
"Status.im theme": "Status.im theme",
"Can't load user settings": "Can't load user settings",
"Server may be unavailable or overloaded": "Server may be unavailable or overloaded",
"Sign out": "Sign out",
"For security, logging out will delete any end-to-end encryption keys from this browser. If you want to be able to decrypt your conversation history from future Riot sessions, please export your room keys for safe-keeping.": "For security, logging out will delete any end-to-end encryption keys from this browser. If you want to be able to decrypt your conversation history from future Riot sessions, please export your room keys for safe-keeping.",
"Success": "Success",
"Your password was successfully changed. You will not receive push notifications on other devices until you log back in to them": "Your password was successfully changed. You will not receive push notifications on other devices until you log back in to them",
Expand Down