-
-
Notifications
You must be signed in to change notification settings - Fork 833
Conversation
* Make the 'delete my data' button not the default * Make it red * Give it a confirmation dialog * Remove the 'cancel' button: what does it mean to cancel an error? In this case, it tried again and almost certainly got the same error. * Remove the top-right 'x' and don't cancel on esc for the same reason. * Move 'send bug report' to a button rather than a 'click here' link * Add a 'refresh' button which, even if it's no more likely to work, will at least look like it's doing something (it's mostly so if you don't have a bug report endpoint, there's still a button other than the one that deletes all your data).
…andling' into dbkr/fix_session_restore_fail_dialog_ux
…andling' into dbkr/fix_session_restore_fail_dialog_ux
* Make it use BaseDialog / DialogButtons (also gives it has a top-right 'x' & escape to cancel works) * Stop misusing the 'danger' CSS class on the buttons. There is nothing dangerous about submitting logs. * Continued campaign against 'Click here' links. Fixes element-hq/element-web#6622
…alog_ux' into dbkr/bug_report_dialog_basedialog
* Make the 'delete my data' button not the default * Make it red * Give it a confirmation dialog * Remove the 'cancel' button: what does it mean to cancel an error? In this case, it tried again and almost certainly got the same error. * Remove the top-right 'x' and don't cancel on esc for the same reason. * Move 'send bug report' to a button rather than a 'click here' link * Add a 'refresh' button which, even if it's no more likely to work, will at least look like it's doing something (it's mostly so if you don't have a bug report endpoint, there's still a button other than the one that deletes all your data).
* Make it use BaseDialog / DialogButtons (also gives it has a top-right 'x' & escape to cancel works) * Stop misusing the 'danger' CSS class on the buttons. There is nothing dangerous about submitting logs. * Continued campaign against 'Click here' links. Fixes element-hq/element-web#6622
74651b9
to
a9b6db3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM otherwise
@@ -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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
@@ -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 true if there is nothing the app can sensibly do if the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this not be false
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth mentioning the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, good spot
hasCancel={false} | ||
> | ||
<button onClick={this._onClearStorageClick} className="danger"> | ||
{ _t("Clear Storage and Sign Out") } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks identical to the if
block apart from the text "Refresh".
OH. I see what this is doing now. Maybe factor out the "Clear Storage..." button and refer to it twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mm, that is probably better. done
|
||
<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("Clearing your browser's storage may fix the problem, but will sign you " + | ||
"out and cause any encrypted chat history to become unreadable.") }</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation could be better here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
…-org/matrix-react-sdk into dbkr/bug_report_dialog_basedialog
…alog_ux' into dbkr/bug_report_dialog_basedialog
escape to cancel works)
about submitting logs.
Fixes element-hq/element-web#6622
Based off #1860 because I keep needing to add things to DialogButtons and it would conflict.