From a38be90a4e8c7ad61ad8e746a294972cf1737ee7 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Mon, 11 May 2020 22:47:26 -0400 Subject: [PATCH 01/24] Extract Alert State, AlertManagerState --- js/src/admin/components/BasicsPage.js | 4 +- js/src/admin/components/MailPage.js | 4 +- js/src/common/Application.js | 20 ++++--- js/src/common/components/Alert.js | 6 ++ js/src/common/components/AlertManager.js | 56 ++----------------- js/src/common/components/Modal.js | 10 ++-- js/src/common/states/AlertManagerState.js | 44 +++++++++++++++ js/src/common/states/AlertState.js | 6 ++ js/src/forum/components/ChangeEmailModal.js | 4 +- .../forum/components/ForgotPasswordModal.js | 2 +- js/src/forum/components/LogInModal.js | 2 +- js/src/forum/components/ReplyComposer.js | 16 +++--- js/src/forum/utils/UserControls.js | 10 ++-- 13 files changed, 98 insertions(+), 86 deletions(-) create mode 100644 js/src/common/states/AlertManagerState.js create mode 100644 js/src/common/states/AlertState.js diff --git a/js/src/admin/components/BasicsPage.js b/js/src/admin/components/BasicsPage.js index 497a0fddbf..d5fce37a4f 100644 --- a/js/src/admin/components/BasicsPage.js +++ b/js/src/admin/components/BasicsPage.js @@ -178,7 +178,7 @@ export default class BasicsPage extends Page { if (this.loading) return; this.loading = true; - app.alerts.dismiss(this.successAlert); + app.alerts.dismiss(this.successAlertKey); const settings = {}; @@ -186,7 +186,7 @@ export default class BasicsPage extends Page { saveSettings(settings) .then(() => { - app.alerts.show((this.successAlert = new Alert({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') }))); + this.successAlertKey = app.alerts.show({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') }); }) .catch(() => {}) .then(() => { diff --git a/js/src/admin/components/MailPage.js b/js/src/admin/components/MailPage.js index cbec021b3b..686ca7d2a1 100644 --- a/js/src/admin/components/MailPage.js +++ b/js/src/admin/components/MailPage.js @@ -196,7 +196,7 @@ export default class MailPage extends Page { if (this.saving || this.sendingTest) return; this.saving = true; - app.alerts.dismiss(this.successAlert); + app.alerts.dismiss(this.successAlertKey); const settings = {}; @@ -204,7 +204,7 @@ export default class MailPage extends Page { saveSettings(settings) .then(() => { - app.alerts.show((this.successAlert = new Alert({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') }))); + this.successAlertKey = app.alerts.show({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') }); }) .catch(() => {}) .then(() => { diff --git a/js/src/common/Application.js b/js/src/common/Application.js index 008b967fd9..d3e9112bc1 100644 --- a/js/src/common/Application.js +++ b/js/src/common/Application.js @@ -23,6 +23,7 @@ import Group from './models/Group'; import Notification from './models/Notification'; import { flattenDeep } from 'lodash-es'; import PageState from './states/PageState'; +import AlertManagerState from './states/AlertManagerState'; /** * The `App` class provides a container for an application, as well as various @@ -139,6 +140,11 @@ export default class Application { */ previous = new PageState(null); + /* + * An object that manages the state of active alerts. + */ + alerts = new AlertManagerState(); + data; title = ''; @@ -175,7 +181,7 @@ export default class Application { mount(basePath = '') { this.modal = m.mount(document.getElementById('modal'), ); - this.alerts = m.mount(document.getElementById('alerts'), ); + m.mount(document.getElementById('alerts'), ); this.drawer = new Drawer(); @@ -310,7 +316,7 @@ export default class Application { } }; - if (this.requestError) this.alerts.dismiss(this.requestError.alert); + if (this.requestError) this.alerts.dismiss(this.requestError.alertKey); // Now make the request. If it's a failure, inspect the error that was // returned and show an alert containing its contents. @@ -354,7 +360,7 @@ export default class Application { // the details property is decoded to transform escaped characters such as '\n' const formattedError = error.response && Array.isArray(error.response.errors) && error.response.errors.map((e) => decodeURI(e.detail)); - error.alert = new Alert({ + error.alertProps = { type: 'error', children, controls: isDebug && [ @@ -362,7 +368,7 @@ export default class Application { Debug , ], - }); + }; try { options.errorHandler(error); @@ -378,7 +384,7 @@ export default class Application { console.groupEnd(); } - this.alerts.show(error.alert); + this.alerts.show(error.alertProps); } deferred.reject(error); @@ -393,8 +399,8 @@ export default class Application { * @param {string[]} [formattedError] * @private */ - showDebug(error, formattedError) { - this.alerts.dismiss(this.requestError.alert); + showDebug(error) { + this.alerts.dismiss(this.requestError.alertKey); this.modal.show(new RequestErrorModal({ error, formattedError })); } diff --git a/js/src/common/components/Alert.js b/js/src/common/components/Alert.js index f26cf4a928..f4953c3d8a 100644 --- a/js/src/common/components/Alert.js +++ b/js/src/common/components/Alert.js @@ -18,6 +18,12 @@ import extract from '../utils/extract'; * All other props will be assigned as attributes on the alert element. */ export default class Alert extends Component { + static initProps(props) { + super.initProps(props); + + if (props.state) Object.assign(props, props.state.props); + } + view() { const attrs = Object.assign({}, this.props); diff --git a/js/src/common/components/AlertManager.js b/js/src/common/components/AlertManager.js index 1592e57fcd..29c78304aa 100644 --- a/js/src/common/components/AlertManager.js +++ b/js/src/common/components/AlertManager.js @@ -7,20 +7,16 @@ import Alert from './Alert'; */ export default class AlertManager extends Component { init() { - /** - * An array of Alert components which are currently showing. - * - * @type {Alert[]} - * @protected - */ - this.components = []; + this.state = this.props.state; } view() { return (
- {this.components.map((component) => ( -
{component}
+ {this.state.activeAlerts.map((alert) => ( +
+ +
))}
); @@ -32,46 +28,4 @@ export default class AlertManager extends Component { // to be retained across route changes. context.retain = true; } - - /** - * Show an Alert in the alerts area. - * - * @param {Alert} component - * @public - */ - show(component) { - if (!(component instanceof Alert)) { - throw new Error('The AlertManager component can only show Alert components'); - } - - component.props.ondismiss = this.dismiss.bind(this, component); - - this.components.push(component); - m.redraw(); - } - - /** - * Dismiss an alert. - * - * @param {Alert} component - * @public - */ - dismiss(component) { - const index = this.components.indexOf(component); - - if (index !== -1) { - this.components.splice(index, 1); - m.redraw(); - } - } - - /** - * Clear all alerts. - * - * @public - */ - clear() { - this.components = []; - m.redraw(); - } } diff --git a/js/src/common/components/Modal.js b/js/src/common/components/Modal.js index 872bb1b6a7..9e64db1129 100644 --- a/js/src/common/components/Modal.js +++ b/js/src/common/components/Modal.js @@ -15,12 +15,12 @@ export default class Modal extends Component { * * @type {Alert} */ - this.alert = null; + this.alertProps = null; } view() { - if (this.alert) { - this.alert.props.dismissible = false; + if (this.alertProps) { + this.alertProps.dismissible = false; } return ( @@ -43,7 +43,7 @@ export default class Modal extends Component {

{this.title()}

- {alert ?
{this.alert}
: ''} + {this.alertProps ?
{Alert.component(this.alertProps)}
: ''} {this.content()} @@ -123,7 +123,7 @@ export default class Modal extends Component { * @param {RequestError} error */ onerror(error) { - this.alert = error.alert; + this.alertProps = error.alertProps; m.redraw(); diff --git a/js/src/common/states/AlertManagerState.js b/js/src/common/states/AlertManagerState.js new file mode 100644 index 0000000000..89e9db33f9 --- /dev/null +++ b/js/src/common/states/AlertManagerState.js @@ -0,0 +1,44 @@ +import AlertState from './AlertState'; + +export default class AlertManagerState { + constructor() { + this.activeAlerts = []; + } + + /** + * Show an Alert in the alerts area. + */ + show(propsOrState) { + const state = propsOrState instanceof AlertState ? propsOrState : new AlertState(propsOrState); + + this.activeAlerts.push(state); + m.redraw(); + + return state.key; + } + + /** + * Dismiss an alert. + */ + dismiss(keyOrState) { + if (!keyOrState) return; + + const key = keyOrState instanceof AlertState ? keyOrState.key : keyOrState; + + let index = this.activeAlerts.indexOf(this.activeAlerts.filter((a) => a.key == key)[0]); + + if (index !== -1) { + this.activeAlerts.splice(index, 1); + m.redraw(); + } + } + /** + * Clear all alerts. + * + * @public + */ + clear() { + this.activeAlerts = []; + m.redraw(); + } +} diff --git a/js/src/common/states/AlertState.js b/js/src/common/states/AlertState.js new file mode 100644 index 0000000000..ffeebd09e8 --- /dev/null +++ b/js/src/common/states/AlertState.js @@ -0,0 +1,6 @@ +export default class AlertState { + constructor(props = {}, key = Date.now()) { + this.props = props; + this.key = key; + } +} diff --git a/js/src/forum/components/ChangeEmailModal.js b/js/src/forum/components/ChangeEmailModal.js index ce1b1592a8..9e9cd22b26 100644 --- a/js/src/forum/components/ChangeEmailModal.js +++ b/js/src/forum/components/ChangeEmailModal.js @@ -121,8 +121,8 @@ export default class ChangeEmailModal extends Modal { } onerror(error) { - if (error.status === 401) { - error.alert.props.children = app.translator.trans('core.forum.change_email.incorrect_password_message'); + if (error.status === 403) { + error.alertProps.children = app.translator.trans('core.forum.change_email.incorrect_password_message'); } super.onerror(error); diff --git a/js/src/forum/components/ForgotPasswordModal.js b/js/src/forum/components/ForgotPasswordModal.js index 4b443e92ec..353c546401 100644 --- a/js/src/forum/components/ForgotPasswordModal.js +++ b/js/src/forum/components/ForgotPasswordModal.js @@ -104,7 +104,7 @@ export default class ForgotPasswordModal extends Modal { onerror(error) { if (error.status === 404) { - error.alert.props.children = app.translator.trans('core.forum.forgot_password.not_found_message'); + error.alertProps.children = app.translator.trans('core.forum.forgot_password.not_found_message'); } super.onerror(error); diff --git a/js/src/forum/components/LogInModal.js b/js/src/forum/components/LogInModal.js index a6d87230ef..0b5b94a9f9 100644 --- a/js/src/forum/components/LogInModal.js +++ b/js/src/forum/components/LogInModal.js @@ -179,7 +179,7 @@ export default class LogInModal extends Modal { onerror(error) { if (error.status === 401) { - error.alert.props.children = app.translator.trans('core.forum.log_in.invalid_login_message'); + error.alertProps.children = app.translator.trans('core.forum.log_in.invalid_login_message'); } super.onerror(error); diff --git a/js/src/forum/components/ReplyComposer.js b/js/src/forum/components/ReplyComposer.js index fd6de3c18c..1034ce4787 100644 --- a/js/src/forum/components/ReplyComposer.js +++ b/js/src/forum/components/ReplyComposer.js @@ -95,22 +95,20 @@ export default class ReplyComposer extends ComposerBody { // Otherwise, we'll create an alert message to inform the user that // their reply has been posted, containing a button which will // transition to their new post when clicked. - let alert; + let alertKey; const viewButton = Button.component({ className: 'Button Button--link', children: app.translator.trans('core.forum.composer_reply.view_button'), onclick: () => { m.route(app.route.post(post)); - app.alerts.dismiss(alert); + app.alerts.dismiss(alertKey); }, }); - app.alerts.show( - (alert = new Alert({ - type: 'success', - children: app.translator.trans('core.forum.composer_reply.posted_message'), - controls: [viewButton], - })) - ); + alertKey = app.alerts.show({ + type: 'success', + children: app.translator.trans('core.forum.composer_reply.posted_message'), + controls: [viewButton], + }); } app.composer.hide(); diff --git a/js/src/forum/utils/UserControls.js b/js/src/forum/utils/UserControls.js index ae18632134..5a3a84771b 100644 --- a/js/src/forum/utils/UserControls.js +++ b/js/src/forum/utils/UserControls.js @@ -134,12 +134,10 @@ export default { error: 'core.forum.user_controls.delete_error_message', }[type]; - app.alerts.show( - new Alert({ - type, - children: app.translator.trans(message, { username, email }), - }) - ); + app.alerts.show({ + type, + children: app.translator.trans(message, { username, email }), + }); }, /** From 24eb104ebbfc0b8ea8ab19904c7d09d757e2489d Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 15 May 2020 20:14:13 -0400 Subject: [PATCH 02/24] Make the backend return the proper error code for incorrect password in ChangeEmailModal --- js/src/forum/components/ChangeEmailModal.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/forum/components/ChangeEmailModal.js b/js/src/forum/components/ChangeEmailModal.js index 9e9cd22b26..2fc2e5c747 100644 --- a/js/src/forum/components/ChangeEmailModal.js +++ b/js/src/forum/components/ChangeEmailModal.js @@ -121,7 +121,7 @@ export default class ChangeEmailModal extends Modal { } onerror(error) { - if (error.status === 403) { + if (error.status === 401) { error.alertProps.children = app.translator.trans('core.forum.change_email.incorrect_password_message'); } From a150ea4f02a0e0343cb967c093b3517e64e1a00f Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 15 May 2020 21:11:51 -0400 Subject: [PATCH 03/24] Modify alert state extraction so that active alerts are stored in an object --- js/src/common/components/Alert.js | 6 ----- js/src/common/components/AlertManager.js | 6 ++--- js/src/common/states/AlertManagerState.js | 27 +++++++++-------------- js/src/common/states/AlertState.js | 5 ++--- 4 files changed, 15 insertions(+), 29 deletions(-) diff --git a/js/src/common/components/Alert.js b/js/src/common/components/Alert.js index f4953c3d8a..f26cf4a928 100644 --- a/js/src/common/components/Alert.js +++ b/js/src/common/components/Alert.js @@ -18,12 +18,6 @@ import extract from '../utils/extract'; * All other props will be assigned as attributes on the alert element. */ export default class Alert extends Component { - static initProps(props) { - super.initProps(props); - - if (props.state) Object.assign(props, props.state.props); - } - view() { const attrs = Object.assign({}, this.props); diff --git a/js/src/common/components/AlertManager.js b/js/src/common/components/AlertManager.js index 29c78304aa..6ca4a0ae90 100644 --- a/js/src/common/components/AlertManager.js +++ b/js/src/common/components/AlertManager.js @@ -13,10 +13,8 @@ export default class AlertManager extends Component { view() { return (
- {this.state.activeAlerts.map((alert) => ( -
- -
+ {Object.entries(this.state.activeAlerts).map(([key, state]) => ( +
{Alert.component({ ...state.attrs, ondismiss: this.state.dismiss.bind(this.state, key) })}
))}
); diff --git a/js/src/common/states/AlertManagerState.js b/js/src/common/states/AlertManagerState.js index 89e9db33f9..be2d3c6de5 100644 --- a/js/src/common/states/AlertManagerState.js +++ b/js/src/common/states/AlertManagerState.js @@ -2,43 +2,38 @@ import AlertState from './AlertState'; export default class AlertManagerState { constructor() { - this.activeAlerts = []; + this.activeAlerts = {}; } /** * Show an Alert in the alerts area. */ - show(propsOrState) { - const state = propsOrState instanceof AlertState ? propsOrState : new AlertState(propsOrState); + show(attrs, key = Date.now()) { + const state = new AlertState(attrs); - this.activeAlerts.push(state); + this.activeAlerts[key] = state; m.redraw(); - return state.key; + return key; } /** * Dismiss an alert. */ - dismiss(keyOrState) { - if (!keyOrState) return; + dismiss(key) { + if (!key || !(key in this.activeAlerts)) return; - const key = keyOrState instanceof AlertState ? keyOrState.key : keyOrState; - - let index = this.activeAlerts.indexOf(this.activeAlerts.filter((a) => a.key == key)[0]); - - if (index !== -1) { - this.activeAlerts.splice(index, 1); - m.redraw(); - } + delete this.activeAlerts[key]; + m.redraw(); } + /** * Clear all alerts. * * @public */ clear() { - this.activeAlerts = []; + this.activeAlerts = {}; m.redraw(); } } diff --git a/js/src/common/states/AlertState.js b/js/src/common/states/AlertState.js index ffeebd09e8..db107c1a83 100644 --- a/js/src/common/states/AlertState.js +++ b/js/src/common/states/AlertState.js @@ -1,6 +1,5 @@ export default class AlertState { - constructor(props = {}, key = Date.now()) { - this.props = props; - this.key = key; + constructor(attrs = {}) { + this.attrs = attrs; } } From 75f15c0a1004072192225491c7f9d894b46843bf Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 15 May 2020 21:13:27 -0400 Subject: [PATCH 04/24] Add missing error.alertKey store --- js/src/common/Application.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/common/Application.js b/js/src/common/Application.js index d3e9112bc1..fe4a713f3f 100644 --- a/js/src/common/Application.js +++ b/js/src/common/Application.js @@ -384,7 +384,7 @@ export default class Application { console.groupEnd(); } - this.alerts.show(error.alertProps); + error.alertKey = this.alerts.show(error.alertProps); } deferred.reject(error); From 1d541b6ca46a98b8a552f0033d0d8e3cc30442d0 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 15 May 2020 21:20:25 -0400 Subject: [PATCH 05/24] Use random integer for alert key instead of date --- js/src/common/states/AlertManagerState.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/js/src/common/states/AlertManagerState.js b/js/src/common/states/AlertManagerState.js index be2d3c6de5..434ba072c8 100644 --- a/js/src/common/states/AlertManagerState.js +++ b/js/src/common/states/AlertManagerState.js @@ -8,7 +8,7 @@ export default class AlertManagerState { /** * Show an Alert in the alerts area. */ - show(attrs, key = Date.now()) { + show(attrs, key = AlertManagerState.genAlertId()) { const state = new AlertState(attrs); this.activeAlerts[key] = state; @@ -36,4 +36,8 @@ export default class AlertManagerState { this.activeAlerts = {}; m.redraw(); } + + static genAlertId() { + return Math.floor(Math.random() * 100000000); // Generate a big random integer to avoid collisions. + } } From 88bf1b209d2b4ab60d3e5dfb516733efe0d36792 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 15 May 2020 21:24:25 -0400 Subject: [PATCH 06/24] Allow passing an alert class in alert.show --- js/src/common/components/AlertManager.js | 4 +++- js/src/common/states/AlertManagerState.js | 5 +++-- js/src/common/states/AlertState.js | 5 ++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/js/src/common/components/AlertManager.js b/js/src/common/components/AlertManager.js index 6ca4a0ae90..25dbbcce1a 100644 --- a/js/src/common/components/AlertManager.js +++ b/js/src/common/components/AlertManager.js @@ -14,7 +14,9 @@ export default class AlertManager extends Component { return (
{Object.entries(this.state.activeAlerts).map(([key, state]) => ( -
{Alert.component({ ...state.attrs, ondismiss: this.state.dismiss.bind(this.state, key) })}
+
+ {state.alertClass.component({ ...state.attrs, ondismiss: this.state.dismiss.bind(this.state, key) })} +
))}
); diff --git a/js/src/common/states/AlertManagerState.js b/js/src/common/states/AlertManagerState.js index 434ba072c8..eb5c583c2c 100644 --- a/js/src/common/states/AlertManagerState.js +++ b/js/src/common/states/AlertManagerState.js @@ -1,3 +1,4 @@ +import Alert from '../components/Alert'; import AlertState from './AlertState'; export default class AlertManagerState { @@ -8,8 +9,8 @@ export default class AlertManagerState { /** * Show an Alert in the alerts area. */ - show(attrs, key = AlertManagerState.genAlertId()) { - const state = new AlertState(attrs); + show(attrs, alertClass = Alert, key = AlertManagerState.genAlertId()) { + const state = new AlertState(attrs, alertClass); this.activeAlerts[key] = state; m.redraw(); diff --git a/js/src/common/states/AlertState.js b/js/src/common/states/AlertState.js index db107c1a83..4b93f653a6 100644 --- a/js/src/common/states/AlertState.js +++ b/js/src/common/states/AlertState.js @@ -1,5 +1,8 @@ +import Alert from '../components/Alert'; + export default class AlertState { - constructor(attrs = {}) { + constructor(attrs = {}, alertClass = Alert) { this.attrs = attrs; + this.alertClass = alertClass; } } From ed7f38211f47a3499e28692e269a6f65262458dc Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 15 May 2020 21:33:31 -0400 Subject: [PATCH 07/24] Change alertProps to alertAttrs --- js/src/common/Application.js | 4 ++-- js/src/common/components/Modal.js | 10 +++++----- js/src/forum/components/ChangeEmailModal.js | 2 +- js/src/forum/components/ForgotPasswordModal.js | 2 +- js/src/forum/components/LogInModal.js | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/js/src/common/Application.js b/js/src/common/Application.js index fe4a713f3f..4f6b06d87b 100644 --- a/js/src/common/Application.js +++ b/js/src/common/Application.js @@ -360,7 +360,7 @@ export default class Application { // the details property is decoded to transform escaped characters such as '\n' const formattedError = error.response && Array.isArray(error.response.errors) && error.response.errors.map((e) => decodeURI(e.detail)); - error.alertProps = { + error.alertAttrs = { type: 'error', children, controls: isDebug && [ @@ -384,7 +384,7 @@ export default class Application { console.groupEnd(); } - error.alertKey = this.alerts.show(error.alertProps); + error.alertKey = this.alerts.show(error.alertAttrs); } deferred.reject(error); diff --git a/js/src/common/components/Modal.js b/js/src/common/components/Modal.js index 9e64db1129..9217ef0189 100644 --- a/js/src/common/components/Modal.js +++ b/js/src/common/components/Modal.js @@ -15,12 +15,12 @@ export default class Modal extends Component { * * @type {Alert} */ - this.alertProps = null; + this.alertAttrs = null; } view() { - if (this.alertProps) { - this.alertProps.dismissible = false; + if (this.alertAttrs) { + this.alertAttrs.dismissible = false; } return ( @@ -43,7 +43,7 @@ export default class Modal extends Component {

{this.title()}

- {this.alertProps ?
{Alert.component(this.alertProps)}
: ''} + {this.alertAttrs ?
{Alert.component(this.alertAttrs)}
: ''} {this.content()} @@ -123,7 +123,7 @@ export default class Modal extends Component { * @param {RequestError} error */ onerror(error) { - this.alertProps = error.alertProps; + this.alertAttrs = error.alertAttrs; m.redraw(); diff --git a/js/src/forum/components/ChangeEmailModal.js b/js/src/forum/components/ChangeEmailModal.js index 2fc2e5c747..0c08731e54 100644 --- a/js/src/forum/components/ChangeEmailModal.js +++ b/js/src/forum/components/ChangeEmailModal.js @@ -122,7 +122,7 @@ export default class ChangeEmailModal extends Modal { onerror(error) { if (error.status === 401) { - error.alertProps.children = app.translator.trans('core.forum.change_email.incorrect_password_message'); + error.alertAttrs.children = app.translator.trans('core.forum.change_email.incorrect_password_message'); } super.onerror(error); diff --git a/js/src/forum/components/ForgotPasswordModal.js b/js/src/forum/components/ForgotPasswordModal.js index 353c546401..22073709f7 100644 --- a/js/src/forum/components/ForgotPasswordModal.js +++ b/js/src/forum/components/ForgotPasswordModal.js @@ -104,7 +104,7 @@ export default class ForgotPasswordModal extends Modal { onerror(error) { if (error.status === 404) { - error.alertProps.children = app.translator.trans('core.forum.forgot_password.not_found_message'); + error.alertAttrs.children = app.translator.trans('core.forum.forgot_password.not_found_message'); } super.onerror(error); diff --git a/js/src/forum/components/LogInModal.js b/js/src/forum/components/LogInModal.js index 0b5b94a9f9..716798f3ef 100644 --- a/js/src/forum/components/LogInModal.js +++ b/js/src/forum/components/LogInModal.js @@ -179,7 +179,7 @@ export default class LogInModal extends Modal { onerror(error) { if (error.status === 401) { - error.alertProps.children = app.translator.trans('core.forum.log_in.invalid_login_message'); + error.alertAttrs.children = app.translator.trans('core.forum.log_in.invalid_login_message'); } super.onerror(error); From c6866a9c9650a05c7940ecce6de5e1b993b0816c Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Sun, 17 May 2020 18:46:40 -0400 Subject: [PATCH 08/24] Revert change of exception thrown when incorrect password on email change --- src/Api/Controller/UpdateUserController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Api/Controller/UpdateUserController.php b/src/Api/Controller/UpdateUserController.php index 6090e6fca1..ae41f182ba 100644 --- a/src/Api/Controller/UpdateUserController.php +++ b/src/Api/Controller/UpdateUserController.php @@ -12,7 +12,7 @@ use Flarum\Api\Serializer\CurrentUserSerializer; use Flarum\Api\Serializer\UserSerializer; use Flarum\User\Command\EditUser; -use Flarum\User\Exception\NotAuthenticatedException; +use Flarum\User\Exception\PermissionDeniedException; use Illuminate\Contracts\Bus\Dispatcher; use Illuminate\Support\Arr; use Psr\Http\Message\ServerRequestInterface; @@ -62,7 +62,7 @@ protected function data(ServerRequestInterface $request, Document $document) $password = Arr::get($request->getParsedBody(), 'meta.password'); if (! $actor->checkPassword($password)) { - throw new NotAuthenticatedException; + throw new PermissionDeniedException; } } From 4e34797ef80880ee027ea1ac2301f1bcbfaeffa4 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Sun, 17 May 2020 21:36:27 -0400 Subject: [PATCH 09/24] Add in missing formattedError parameter --- js/src/common/Application.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/common/Application.js b/js/src/common/Application.js index 4f6b06d87b..bf9f7a93e8 100644 --- a/js/src/common/Application.js +++ b/js/src/common/Application.js @@ -399,7 +399,7 @@ export default class Application { * @param {string[]} [formattedError] * @private */ - showDebug(error) { + showDebug(error, formattedError) { this.alerts.dismiss(this.requestError.alertKey); this.modal.show(new RequestErrorModal({ error, formattedError })); From 887803e4af4c597b2f9d349effec3693b3d9e824 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Sat, 23 May 2020 04:47:30 -0400 Subject: [PATCH 10/24] Require an alert state instance to show an alert, put key back into alert state. --- js/src/common/states/AlertManagerState.js | 12 ++---------- js/src/common/states/AlertState.js | 9 +++++++-- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/js/src/common/states/AlertManagerState.js b/js/src/common/states/AlertManagerState.js index eb5c583c2c..04cc92340c 100644 --- a/js/src/common/states/AlertManagerState.js +++ b/js/src/common/states/AlertManagerState.js @@ -1,6 +1,3 @@ -import Alert from '../components/Alert'; -import AlertState from './AlertState'; - export default class AlertManagerState { constructor() { this.activeAlerts = {}; @@ -9,10 +6,9 @@ export default class AlertManagerState { /** * Show an Alert in the alerts area. */ - show(attrs, alertClass = Alert, key = AlertManagerState.genAlertId()) { - const state = new AlertState(attrs, alertClass); + show(state) { - this.activeAlerts[key] = state; + this.activeAlerts[state.key] = state; m.redraw(); return key; @@ -37,8 +33,4 @@ export default class AlertManagerState { this.activeAlerts = {}; m.redraw(); } - - static genAlertId() { - return Math.floor(Math.random() * 100000000); // Generate a big random integer to avoid collisions. - } } diff --git a/js/src/common/states/AlertState.js b/js/src/common/states/AlertState.js index 4b93f653a6..ffe81578e8 100644 --- a/js/src/common/states/AlertState.js +++ b/js/src/common/states/AlertState.js @@ -1,8 +1,13 @@ import Alert from '../components/Alert'; export default class AlertState { - constructor(attrs = {}, alertClass = Alert) { - this.attrs = attrs; + constructor(alertClass = Alert, attrs = {}, alertKey = AlertState.genAlertId()) { this.alertClass = alertClass; + this.attrs = attrs; + this.alertKey = alertKey; + } + + static genAlertId() { + return Math.floor(Math.random() * 100000000); // Generate a big random integer to avoid collisions. } } From 2c915e711c5fffa4abdf87aa3ad51506cba41bf5 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Sat, 23 May 2020 04:47:45 -0400 Subject: [PATCH 11/24] prettify --- js/src/common/states/AlertManagerState.js | 1 - 1 file changed, 1 deletion(-) diff --git a/js/src/common/states/AlertManagerState.js b/js/src/common/states/AlertManagerState.js index 04cc92340c..84a5c5bc92 100644 --- a/js/src/common/states/AlertManagerState.js +++ b/js/src/common/states/AlertManagerState.js @@ -7,7 +7,6 @@ export default class AlertManagerState { * Show an Alert in the alerts area. */ show(state) { - this.activeAlerts[state.key] = state; m.redraw(); From 22adc269b01f0980d8259fbeee0fb309db8d8d2a Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Sat, 23 May 2020 04:56:31 -0400 Subject: [PATCH 12/24] Show alerts by passing an AlertState --- js/src/admin/components/BasicsPage.js | 4 ++-- js/src/admin/components/MailPage.js | 3 ++- js/src/common/Application.js | 11 ++++++----- js/src/common/states/AlertState.js | 4 ++-- js/src/forum/components/ReplyComposer.js | 5 +++-- js/src/forum/utils/UserControls.js | 5 +++-- 6 files changed, 18 insertions(+), 14 deletions(-) diff --git a/js/src/admin/components/BasicsPage.js b/js/src/admin/components/BasicsPage.js index d5fce37a4f..e56a511bd1 100644 --- a/js/src/admin/components/BasicsPage.js +++ b/js/src/admin/components/BasicsPage.js @@ -2,7 +2,7 @@ import Page from '../../common/components/Page'; import FieldSet from '../../common/components/FieldSet'; import Select from '../../common/components/Select'; import Button from '../../common/components/Button'; -import Alert from '../../common/components/Alert'; +import AlertState from '../../common/states/AlertState'; import saveSettings from '../utils/saveSettings'; import ItemList from '../../common/utils/ItemList'; import Switch from '../../common/components/Switch'; @@ -186,7 +186,7 @@ export default class BasicsPage extends Page { saveSettings(settings) .then(() => { - this.successAlertKey = app.alerts.show({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') }); + this.successAlertKey = app.alerts.show(new AlertState({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') })); }) .catch(() => {}) .then(() => { diff --git a/js/src/admin/components/MailPage.js b/js/src/admin/components/MailPage.js index 686ca7d2a1..520ef7046f 100644 --- a/js/src/admin/components/MailPage.js +++ b/js/src/admin/components/MailPage.js @@ -5,6 +5,7 @@ import Alert from '../../common/components/Alert'; import Select from '../../common/components/Select'; import LoadingIndicator from '../../common/components/LoadingIndicator'; import saveSettings from '../utils/saveSettings'; +import AlertState from '../../common/states/AlertState'; export default class MailPage extends Page { init() { @@ -204,7 +205,7 @@ export default class MailPage extends Page { saveSettings(settings) .then(() => { - this.successAlertKey = app.alerts.show({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') }); + this.successAlertKey = app.alerts.show(new AlertState({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') })); }) .catch(() => {}) .then(() => { diff --git a/js/src/common/Application.js b/js/src/common/Application.js index bf9f7a93e8..a94741ae51 100644 --- a/js/src/common/Application.js +++ b/js/src/common/Application.js @@ -24,6 +24,7 @@ import Notification from './models/Notification'; import { flattenDeep } from 'lodash-es'; import PageState from './states/PageState'; import AlertManagerState from './states/AlertManagerState'; +import AlertState from './states/AlertState'; /** * The `App` class provides a container for an application, as well as various @@ -316,7 +317,7 @@ export default class Application { } }; - if (this.requestError) this.alerts.dismiss(this.requestError.alertKey); + if (this.requestError) this.alerts.dismiss(this.requestError.alert.key); // Now make the request. If it's a failure, inspect the error that was // returned and show an alert containing its contents. @@ -360,7 +361,7 @@ export default class Application { // the details property is decoded to transform escaped characters such as '\n' const formattedError = error.response && Array.isArray(error.response.errors) && error.response.errors.map((e) => decodeURI(e.detail)); - error.alertAttrs = { + error.alert = new AlertState({ type: 'error', children, controls: isDebug && [ @@ -368,7 +369,7 @@ export default class Application { Debug , ], - }; + }); try { options.errorHandler(error); @@ -384,7 +385,7 @@ export default class Application { console.groupEnd(); } - error.alertKey = this.alerts.show(error.alertAttrs); + this.alerts.show(error.alert); } deferred.reject(error); @@ -400,7 +401,7 @@ export default class Application { * @private */ showDebug(error, formattedError) { - this.alerts.dismiss(this.requestError.alertKey); + this.alerts.dismiss(this.requestError.alert.key); this.modal.show(new RequestErrorModal({ error, formattedError })); } diff --git a/js/src/common/states/AlertState.js b/js/src/common/states/AlertState.js index ffe81578e8..a4ef825f0c 100644 --- a/js/src/common/states/AlertState.js +++ b/js/src/common/states/AlertState.js @@ -1,9 +1,9 @@ import Alert from '../components/Alert'; export default class AlertState { - constructor(alertClass = Alert, attrs = {}, alertKey = AlertState.genAlertId()) { - this.alertClass = alertClass; + constructor(attrs = {}, alertClass = Alert, alertKey = AlertState.genAlertId()) { this.attrs = attrs; + this.alertClass = alertClass; this.alertKey = alertKey; } diff --git a/js/src/forum/components/ReplyComposer.js b/js/src/forum/components/ReplyComposer.js index 1034ce4787..7b1aa86b24 100644 --- a/js/src/forum/components/ReplyComposer.js +++ b/js/src/forum/components/ReplyComposer.js @@ -3,6 +3,7 @@ import Alert from '../../common/components/Alert'; import Button from '../../common/components/Button'; import icon from '../../common/helpers/icon'; import extractText from '../../common/utils/extractText'; +import AlertState from '../../common/states/AlertState'; function minimizeComposerIfFullScreen(e) { if (app.composer.isFullScreen()) { @@ -104,11 +105,11 @@ export default class ReplyComposer extends ComposerBody { app.alerts.dismiss(alertKey); }, }); - alertKey = app.alerts.show({ + alertKey = app.alerts.show(new AlertState({ type: 'success', children: app.translator.trans('core.forum.composer_reply.posted_message'), controls: [viewButton], - }); + })); } app.composer.hide(); diff --git a/js/src/forum/utils/UserControls.js b/js/src/forum/utils/UserControls.js index 5a3a84771b..099e2c2bf1 100644 --- a/js/src/forum/utils/UserControls.js +++ b/js/src/forum/utils/UserControls.js @@ -4,6 +4,7 @@ import Separator from '../../common/components/Separator'; import EditUserModal from '../components/EditUserModal'; import UserPage from '../components/UserPage'; import ItemList from '../../common/utils/ItemList'; +import AlertState from '../../common/states/AlertState'; /** * The `UserControls` utility constructs a list of buttons for a user which @@ -134,10 +135,10 @@ export default { error: 'core.forum.user_controls.delete_error_message', }[type]; - app.alerts.show({ + app.alerts.show(new AlertState({ type, children: app.translator.trans(message, { username, email }), - }); + })); }, /** From 35afd50af1d010bb38d06bb1faff105316c14242 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Sat, 23 May 2020 04:58:04 -0400 Subject: [PATCH 13/24] Prettify? --- js/src/admin/components/BasicsPage.js | 4 +++- js/src/admin/components/MailPage.js | 4 +++- js/src/common/states/AlertManagerState.js | 2 +- js/src/forum/components/ReplyComposer.js | 12 +++++++----- js/src/forum/utils/UserControls.js | 10 ++++++---- 5 files changed, 20 insertions(+), 12 deletions(-) diff --git a/js/src/admin/components/BasicsPage.js b/js/src/admin/components/BasicsPage.js index e56a511bd1..11c35d0d75 100644 --- a/js/src/admin/components/BasicsPage.js +++ b/js/src/admin/components/BasicsPage.js @@ -186,7 +186,9 @@ export default class BasicsPage extends Page { saveSettings(settings) .then(() => { - this.successAlertKey = app.alerts.show(new AlertState({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') })); + this.successAlertKey = app.alerts.show( + new AlertState({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') }) + ); }) .catch(() => {}) .then(() => { diff --git a/js/src/admin/components/MailPage.js b/js/src/admin/components/MailPage.js index 520ef7046f..9914ba4afa 100644 --- a/js/src/admin/components/MailPage.js +++ b/js/src/admin/components/MailPage.js @@ -205,7 +205,9 @@ export default class MailPage extends Page { saveSettings(settings) .then(() => { - this.successAlertKey = app.alerts.show(new AlertState({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') })); + this.successAlertKey = app.alerts.show( + new AlertState({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') }) + ); }) .catch(() => {}) .then(() => { diff --git a/js/src/common/states/AlertManagerState.js b/js/src/common/states/AlertManagerState.js index 84a5c5bc92..d5537e329e 100644 --- a/js/src/common/states/AlertManagerState.js +++ b/js/src/common/states/AlertManagerState.js @@ -10,7 +10,7 @@ export default class AlertManagerState { this.activeAlerts[state.key] = state; m.redraw(); - return key; + return state.key; } /** diff --git a/js/src/forum/components/ReplyComposer.js b/js/src/forum/components/ReplyComposer.js index 7b1aa86b24..984f425ec8 100644 --- a/js/src/forum/components/ReplyComposer.js +++ b/js/src/forum/components/ReplyComposer.js @@ -105,11 +105,13 @@ export default class ReplyComposer extends ComposerBody { app.alerts.dismiss(alertKey); }, }); - alertKey = app.alerts.show(new AlertState({ - type: 'success', - children: app.translator.trans('core.forum.composer_reply.posted_message'), - controls: [viewButton], - })); + alertKey = app.alerts.show( + new AlertState({ + type: 'success', + children: app.translator.trans('core.forum.composer_reply.posted_message'), + controls: [viewButton], + }) + ); } app.composer.hide(); diff --git a/js/src/forum/utils/UserControls.js b/js/src/forum/utils/UserControls.js index 099e2c2bf1..a27076947c 100644 --- a/js/src/forum/utils/UserControls.js +++ b/js/src/forum/utils/UserControls.js @@ -135,10 +135,12 @@ export default { error: 'core.forum.user_controls.delete_error_message', }[type]; - app.alerts.show(new AlertState({ - type, - children: app.translator.trans(message, { username, email }), - })); + app.alerts.show( + new AlertState({ + type, + children: app.translator.trans(message, { username, email }), + }) + ); }, /** From 7b01596d8c3fc0c680b2f4ffd8a76f15daad906c Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Thu, 28 May 2020 16:43:31 -0400 Subject: [PATCH 14/24] Encapsulate alertstate, alertmanagerstate --- js/src/common/Application.js | 4 ++-- js/src/common/components/AlertManager.js | 4 ++-- js/src/common/states/AlertManagerState.js | 10 +++++++--- js/src/common/states/AlertState.js | 20 ++++++++++++++++++++ 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/js/src/common/Application.js b/js/src/common/Application.js index a94741ae51..7e518b277b 100644 --- a/js/src/common/Application.js +++ b/js/src/common/Application.js @@ -317,7 +317,7 @@ export default class Application { } }; - if (this.requestError) this.alerts.dismiss(this.requestError.alert.key); + if (this.requestError) this.alerts.dismiss(this.requestError.alert.getKey()); // Now make the request. If it's a failure, inspect the error that was // returned and show an alert containing its contents. @@ -401,7 +401,7 @@ export default class Application { * @private */ showDebug(error, formattedError) { - this.alerts.dismiss(this.requestError.alert.key); + this.alerts.dismiss(this.requestError.alert.getKey()); this.modal.show(new RequestErrorModal({ error, formattedError })); } diff --git a/js/src/common/components/AlertManager.js b/js/src/common/components/AlertManager.js index 25dbbcce1a..344f2d558d 100644 --- a/js/src/common/components/AlertManager.js +++ b/js/src/common/components/AlertManager.js @@ -13,9 +13,9 @@ export default class AlertManager extends Component { view() { return (
- {Object.entries(this.state.activeAlerts).map(([key, state]) => ( + {Object.entries(this.state.getActiveAlerts()).map(([key, state]) => (
- {state.alertClass.component({ ...state.attrs, ondismiss: this.state.dismiss.bind(this.state, key) })} + {state.getClass().component({ ...state.getAttrs(), ondismiss: this.state.dismiss.bind(this.state, key) })}
))}
diff --git a/js/src/common/states/AlertManagerState.js b/js/src/common/states/AlertManagerState.js index d5537e329e..94a6bdf78f 100644 --- a/js/src/common/states/AlertManagerState.js +++ b/js/src/common/states/AlertManagerState.js @@ -3,14 +3,18 @@ export default class AlertManagerState { this.activeAlerts = {}; } + getActiveAlerts() { + return this.activeAlerts; + } + /** * Show an Alert in the alerts area. */ - show(state) { - this.activeAlerts[state.key] = state; + show(alert) { + this.activeAlerts[alert.getKey()] = state; m.redraw(); - return state.key; + return alert.getKey(); } /** diff --git a/js/src/common/states/AlertState.js b/js/src/common/states/AlertState.js index a4ef825f0c..208ea327c4 100644 --- a/js/src/common/states/AlertState.js +++ b/js/src/common/states/AlertState.js @@ -7,6 +7,26 @@ export default class AlertState { this.alertKey = alertKey; } + getAttrs() { + return this.attrs; + } + + setAttrs(attrs) { + this.attrs = attrs; + } + + getClass() { + return this.alertClass; + } + + setClass(alertClass) { + this.alertClass = alertClass; + } + + getKey() { + return this.key; + } + static genAlertId() { return Math.floor(Math.random() * 100000000); // Generate a big random integer to avoid collisions. } From 5f394d7c2e46b952bb24ca257463968587b3e3d6 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 19 Jun 2020 17:58:49 -0400 Subject: [PATCH 15/24] Take out unnecessary revert --- src/Api/Controller/UpdateUserController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Api/Controller/UpdateUserController.php b/src/Api/Controller/UpdateUserController.php index ae41f182ba..6090e6fca1 100644 --- a/src/Api/Controller/UpdateUserController.php +++ b/src/Api/Controller/UpdateUserController.php @@ -12,7 +12,7 @@ use Flarum\Api\Serializer\CurrentUserSerializer; use Flarum\Api\Serializer\UserSerializer; use Flarum\User\Command\EditUser; -use Flarum\User\Exception\PermissionDeniedException; +use Flarum\User\Exception\NotAuthenticatedException; use Illuminate\Contracts\Bus\Dispatcher; use Illuminate\Support\Arr; use Psr\Http\Message\ServerRequestInterface; @@ -62,7 +62,7 @@ protected function data(ServerRequestInterface $request, Document $document) $password = Arr::get($request->getParsedBody(), 'meta.password'); if (! $actor->checkPassword($password)) { - throw new PermissionDeniedException; + throw new NotAuthenticatedException; } } From bcaae0b341c33867231f85b02cf8217e552f915c Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Fri, 19 Jun 2020 18:19:27 -0400 Subject: [PATCH 16/24] Use alert state as internal API only --- js/src/admin/components/BasicsPage.js | 10 +++++----- js/src/admin/components/MailPage.js | 17 ++++++++-------- js/src/common/Application.js | 22 +++++++++------------ js/src/common/states/AlertManagerState.js | 10 +++++++--- js/src/common/states/AlertState.js | 19 +----------------- js/src/forum/components/EditPostComposer.js | 12 +++++------ js/src/forum/components/ReplyComposer.js | 14 +++++-------- js/src/forum/utils/UserControls.js | 12 ++++------- 8 files changed, 45 insertions(+), 71 deletions(-) diff --git a/js/src/admin/components/BasicsPage.js b/js/src/admin/components/BasicsPage.js index 11c35d0d75..68f9ba1425 100644 --- a/js/src/admin/components/BasicsPage.js +++ b/js/src/admin/components/BasicsPage.js @@ -2,7 +2,6 @@ import Page from '../../common/components/Page'; import FieldSet from '../../common/components/FieldSet'; import Select from '../../common/components/Select'; import Button from '../../common/components/Button'; -import AlertState from '../../common/states/AlertState'; import saveSettings from '../utils/saveSettings'; import ItemList from '../../common/utils/ItemList'; import Switch from '../../common/components/Switch'; @@ -178,7 +177,7 @@ export default class BasicsPage extends Page { if (this.loading) return; this.loading = true; - app.alerts.dismiss(this.successAlertKey); + app.alerts.dismiss(this.successAlert); const settings = {}; @@ -186,9 +185,10 @@ export default class BasicsPage extends Page { saveSettings(settings) .then(() => { - this.successAlertKey = app.alerts.show( - new AlertState({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') }) - ); + this.successAlert = app.alerts.show({ + type: 'success', + children: app.translator.trans('core.admin.basics.saved_message'), + }); }) .catch(() => {}) .then(() => { diff --git a/js/src/admin/components/MailPage.js b/js/src/admin/components/MailPage.js index 9914ba4afa..aed503846d 100644 --- a/js/src/admin/components/MailPage.js +++ b/js/src/admin/components/MailPage.js @@ -5,7 +5,6 @@ import Alert from '../../common/components/Alert'; import Select from '../../common/components/Select'; import LoadingIndicator from '../../common/components/LoadingIndicator'; import saveSettings from '../utils/saveSettings'; -import AlertState from '../../common/states/AlertState'; export default class MailPage extends Page { init() { @@ -180,9 +179,10 @@ export default class MailPage extends Page { }) .then((response) => { this.sendingTest = false; - app.alerts.show( - (this.testEmailSuccessAlert = new Alert({ type: 'success', children: app.translator.trans('core.admin.email.send_test_mail_success') })) - ); + this.testEmailSuccessAlert = app.alerts.show({ + type: 'success', + children: app.translator.trans('core.admin.email.send_test_mail_success'), + }); }) .catch((error) => { this.sendingTest = false; @@ -197,7 +197,7 @@ export default class MailPage extends Page { if (this.saving || this.sendingTest) return; this.saving = true; - app.alerts.dismiss(this.successAlertKey); + app.alerts.dismiss(this.successAlerts); const settings = {}; @@ -205,9 +205,10 @@ export default class MailPage extends Page { saveSettings(settings) .then(() => { - this.successAlertKey = app.alerts.show( - new AlertState({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message') }) - ); + this.successAlerts = app.alerts.show({ + type: 'success', + children: app.translator.trans('core.admin.basics.saved_message'), + }); }) .catch(() => {}) .then(() => { diff --git a/js/src/common/Application.js b/js/src/common/Application.js index 7e518b277b..79c35f73bd 100644 --- a/js/src/common/Application.js +++ b/js/src/common/Application.js @@ -1,5 +1,4 @@ import ItemList from './utils/ItemList'; -import Alert from './components/Alert'; import Button from './components/Button'; import ModalManager from './components/ModalManager'; import AlertManager from './components/AlertManager'; @@ -24,7 +23,6 @@ import Notification from './models/Notification'; import { flattenDeep } from 'lodash-es'; import PageState from './states/PageState'; import AlertManagerState from './states/AlertManagerState'; -import AlertState from './states/AlertState'; /** * The `App` class provides a container for an application, as well as various @@ -111,13 +109,13 @@ export default class Application { booted = false; /** - * An Alert that was shown as a result of an AJAX request error. If present, - * it will be dismissed on the next successful request. + * The key for an Alert that was shown as a result of an AJAX request error. + * If present, it will be dismissed on the next successful request. * - * @type {null|Alert} + * @type {int} * @private */ - requestError = null; + requestErrorAlert = null; /** * The page the app is currently on. @@ -317,7 +315,7 @@ export default class Application { } }; - if (this.requestError) this.alerts.dismiss(this.requestError.alert.getKey()); + if (this.requestErrorAlert) this.alerts.dismiss(this.requestErrorAlert); // Now make the request. If it's a failure, inspect the error that was // returned and show an alert containing its contents. @@ -326,8 +324,6 @@ export default class Application { m.request(options).then( (response) => deferred.resolve(response), (error) => { - this.requestError = error; - let children; switch (error.status) { @@ -361,7 +357,7 @@ export default class Application { // the details property is decoded to transform escaped characters such as '\n' const formattedError = error.response && Array.isArray(error.response.errors) && error.response.errors.map((e) => decodeURI(e.detail)); - error.alert = new AlertState({ + error.alert = { type: 'error', children, controls: isDebug && [ @@ -369,7 +365,7 @@ export default class Application { Debug , ], - }); + }; try { options.errorHandler(error); @@ -385,7 +381,7 @@ export default class Application { console.groupEnd(); } - this.alerts.show(error.alert); + this.requestErrorAlert = this.alerts.show(error.alert); } deferred.reject(error); @@ -401,7 +397,7 @@ export default class Application { * @private */ showDebug(error, formattedError) { - this.alerts.dismiss(this.requestError.alert.getKey()); + this.alerts.dismiss(this.requestErrorAlert); this.modal.show(new RequestErrorModal({ error, formattedError })); } diff --git a/js/src/common/states/AlertManagerState.js b/js/src/common/states/AlertManagerState.js index 94a6bdf78f..8cf9349cdc 100644 --- a/js/src/common/states/AlertManagerState.js +++ b/js/src/common/states/AlertManagerState.js @@ -1,6 +1,9 @@ +import AlertState from '../states/AlertState'; + export default class AlertManagerState { constructor() { this.activeAlerts = {}; + this.alertId = 0; } getActiveAlerts() { @@ -10,11 +13,12 @@ export default class AlertManagerState { /** * Show an Alert in the alerts area. */ - show(alert) { - this.activeAlerts[alert.getKey()] = state; + show(attrs, alertClass) { + const alert = new AlertState(attrs, alertClass); + this.activeAlerts[++this.alertId] = alert; m.redraw(); - return alert.getKey(); + return this.alertId; } /** diff --git a/js/src/common/states/AlertState.js b/js/src/common/states/AlertState.js index 208ea327c4..446531ec21 100644 --- a/js/src/common/states/AlertState.js +++ b/js/src/common/states/AlertState.js @@ -1,33 +1,16 @@ import Alert from '../components/Alert'; export default class AlertState { - constructor(attrs = {}, alertClass = Alert, alertKey = AlertState.genAlertId()) { + constructor(attrs = {}, alertClass = Alert) { this.attrs = attrs; this.alertClass = alertClass; - this.alertKey = alertKey; } getAttrs() { return this.attrs; } - setAttrs(attrs) { - this.attrs = attrs; - } - getClass() { return this.alertClass; } - - setClass(alertClass) { - this.alertClass = alertClass; - } - - getKey() { - return this.key; - } - - static genAlertId() { - return Math.floor(Math.random() * 100000000); // Generate a big random integer to avoid collisions. - } } diff --git a/js/src/forum/components/EditPostComposer.js b/js/src/forum/components/EditPostComposer.js index 86b9703e57..1602b4dd88 100644 --- a/js/src/forum/components/EditPostComposer.js +++ b/js/src/forum/components/EditPostComposer.js @@ -101,13 +101,11 @@ export default class EditPostComposer extends ComposerBody { app.alerts.dismiss(alert); }, }); - app.alerts.show( - (alert = new Alert({ - type: 'success', - children: app.translator.trans('core.forum.composer_edit.edited_message'), - controls: [viewButton], - })) - ); + alert = app.alerts.show({ + type: 'success', + children: app.translator.trans('core.forum.composer_edit.edited_message'), + controls: [viewButton], + }); } app.composer.hide(); diff --git a/js/src/forum/components/ReplyComposer.js b/js/src/forum/components/ReplyComposer.js index 984f425ec8..327bc3b988 100644 --- a/js/src/forum/components/ReplyComposer.js +++ b/js/src/forum/components/ReplyComposer.js @@ -1,9 +1,7 @@ import ComposerBody from './ComposerBody'; -import Alert from '../../common/components/Alert'; import Button from '../../common/components/Button'; import icon from '../../common/helpers/icon'; import extractText from '../../common/utils/extractText'; -import AlertState from '../../common/states/AlertState'; function minimizeComposerIfFullScreen(e) { if (app.composer.isFullScreen()) { @@ -105,13 +103,11 @@ export default class ReplyComposer extends ComposerBody { app.alerts.dismiss(alertKey); }, }); - alertKey = app.alerts.show( - new AlertState({ - type: 'success', - children: app.translator.trans('core.forum.composer_reply.posted_message'), - controls: [viewButton], - }) - ); + alertKey = app.alerts.show({ + type: 'success', + children: app.translator.trans('core.forum.composer_reply.posted_message'), + controls: [viewButton], + }); } app.composer.hide(); diff --git a/js/src/forum/utils/UserControls.js b/js/src/forum/utils/UserControls.js index a27076947c..7271f1fc55 100644 --- a/js/src/forum/utils/UserControls.js +++ b/js/src/forum/utils/UserControls.js @@ -1,10 +1,8 @@ -import Alert from '../../common/components/Alert'; import Button from '../../common/components/Button'; import Separator from '../../common/components/Separator'; import EditUserModal from '../components/EditUserModal'; import UserPage from '../components/UserPage'; import ItemList from '../../common/utils/ItemList'; -import AlertState from '../../common/states/AlertState'; /** * The `UserControls` utility constructs a list of buttons for a user which @@ -135,12 +133,10 @@ export default { error: 'core.forum.user_controls.delete_error_message', }[type]; - app.alerts.show( - new AlertState({ - type, - children: app.translator.trans(message, { username, email }), - }) - ); + app.alerts.show({ + type, + children: app.translator.trans(message, { username, email }), + }); }, /** From ac48153cf4222756399de2b624679ff8e10be514 Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Fri, 26 Jun 2020 18:19:25 +0200 Subject: [PATCH 17/24] Make AlertState obsolete --- js/src/common/components/AlertManager.js | 4 ++-- js/src/common/states/AlertManagerState.js | 7 ++----- js/src/common/states/AlertState.js | 16 ---------------- 3 files changed, 4 insertions(+), 23 deletions(-) delete mode 100644 js/src/common/states/AlertState.js diff --git a/js/src/common/components/AlertManager.js b/js/src/common/components/AlertManager.js index 344f2d558d..b33b6c01a9 100644 --- a/js/src/common/components/AlertManager.js +++ b/js/src/common/components/AlertManager.js @@ -13,9 +13,9 @@ export default class AlertManager extends Component { view() { return (
- {Object.entries(this.state.getActiveAlerts()).map(([key, state]) => ( + {Object.entries(this.state.getActiveAlerts()).map(([key, alert]) => (
- {state.getClass().component({ ...state.getAttrs(), ondismiss: this.state.dismiss.bind(this.state, key) })} + {(alert.type || Alert).component({ ...alert.attrs, ondismiss: this.state.dismiss.bind(this.state, key) })}
))}
diff --git a/js/src/common/states/AlertManagerState.js b/js/src/common/states/AlertManagerState.js index 8cf9349cdc..a0d701e997 100644 --- a/js/src/common/states/AlertManagerState.js +++ b/js/src/common/states/AlertManagerState.js @@ -1,5 +1,3 @@ -import AlertState from '../states/AlertState'; - export default class AlertManagerState { constructor() { this.activeAlerts = {}; @@ -13,9 +11,8 @@ export default class AlertManagerState { /** * Show an Alert in the alerts area. */ - show(attrs, alertClass) { - const alert = new AlertState(attrs, alertClass); - this.activeAlerts[++this.alertId] = alert; + show(attrs, type) { + this.activeAlerts[++this.alertId] = { type, attrs }; m.redraw(); return this.alertId; diff --git a/js/src/common/states/AlertState.js b/js/src/common/states/AlertState.js deleted file mode 100644 index 446531ec21..0000000000 --- a/js/src/common/states/AlertState.js +++ /dev/null @@ -1,16 +0,0 @@ -import Alert from '../components/Alert'; - -export default class AlertState { - constructor(attrs = {}, alertClass = Alert) { - this.attrs = attrs; - this.alertClass = alertClass; - } - - getAttrs() { - return this.attrs; - } - - getClass() { - return this.alertClass; - } -} From 03d1bf66c34131d82c1d69eb8fae014281560351 Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Fri, 26 Jun 2020 18:20:05 +0200 Subject: [PATCH 18/24] Clean up variable names, doc blocks and imports --- js/src/admin/components/MailPage.js | 4 ++-- js/src/common/components/Modal.js | 4 ++-- js/src/forum/components/EditPostComposer.js | 1 - js/src/forum/components/ReplyComposer.js | 6 +++--- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/js/src/admin/components/MailPage.js b/js/src/admin/components/MailPage.js index aed503846d..13ad25dfcd 100644 --- a/js/src/admin/components/MailPage.js +++ b/js/src/admin/components/MailPage.js @@ -197,7 +197,7 @@ export default class MailPage extends Page { if (this.saving || this.sendingTest) return; this.saving = true; - app.alerts.dismiss(this.successAlerts); + app.alerts.dismiss(this.successAlert); const settings = {}; @@ -205,7 +205,7 @@ export default class MailPage extends Page { saveSettings(settings) .then(() => { - this.successAlerts = app.alerts.show({ + this.successAlert = app.alerts.show({ type: 'success', children: app.translator.trans('core.admin.basics.saved_message'), }); diff --git a/js/src/common/components/Modal.js b/js/src/common/components/Modal.js index 9217ef0189..f0f4de453f 100644 --- a/js/src/common/components/Modal.js +++ b/js/src/common/components/Modal.js @@ -11,9 +11,9 @@ import Button from './Button'; export default class Modal extends Component { init() { /** - * An alert component to show below the header. + * Attributes for an alert component to show below the header. * - * @type {Alert} + * @type {object} */ this.alertAttrs = null; } diff --git a/js/src/forum/components/EditPostComposer.js b/js/src/forum/components/EditPostComposer.js index 1602b4dd88..258e8ecc30 100644 --- a/js/src/forum/components/EditPostComposer.js +++ b/js/src/forum/components/EditPostComposer.js @@ -1,5 +1,4 @@ import ComposerBody from './ComposerBody'; -import Alert from '../../common/components/Alert'; import Button from '../../common/components/Button'; import icon from '../../common/helpers/icon'; diff --git a/js/src/forum/components/ReplyComposer.js b/js/src/forum/components/ReplyComposer.js index 327bc3b988..af942d1861 100644 --- a/js/src/forum/components/ReplyComposer.js +++ b/js/src/forum/components/ReplyComposer.js @@ -94,16 +94,16 @@ export default class ReplyComposer extends ComposerBody { // Otherwise, we'll create an alert message to inform the user that // their reply has been posted, containing a button which will // transition to their new post when clicked. - let alertKey; + let alert; const viewButton = Button.component({ className: 'Button Button--link', children: app.translator.trans('core.forum.composer_reply.view_button'), onclick: () => { m.route(app.route.post(post)); - app.alerts.dismiss(alertKey); + app.alerts.dismiss(alert); }, }); - alertKey = app.alerts.show({ + alert = app.alerts.show({ type: 'success', children: app.translator.trans('core.forum.composer_reply.posted_message'), controls: [viewButton], From cc0b2281e50a56c4be91b4e149f1c90fbaf6191a Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Fri, 26 Jun 2020 18:44:43 +0200 Subject: [PATCH 19/24] Fix alert attrs for modal errors --- js/src/common/components/Modal.js | 2 +- js/src/forum/components/ChangeEmailModal.js | 2 +- js/src/forum/components/ForgotPasswordModal.js | 2 +- js/src/forum/components/LogInModal.js | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/js/src/common/components/Modal.js b/js/src/common/components/Modal.js index f0f4de453f..86cef30319 100644 --- a/js/src/common/components/Modal.js +++ b/js/src/common/components/Modal.js @@ -123,7 +123,7 @@ export default class Modal extends Component { * @param {RequestError} error */ onerror(error) { - this.alertAttrs = error.alertAttrs; + this.alertAttrs = error.alert; m.redraw(); diff --git a/js/src/forum/components/ChangeEmailModal.js b/js/src/forum/components/ChangeEmailModal.js index 0c08731e54..ff45dbedef 100644 --- a/js/src/forum/components/ChangeEmailModal.js +++ b/js/src/forum/components/ChangeEmailModal.js @@ -122,7 +122,7 @@ export default class ChangeEmailModal extends Modal { onerror(error) { if (error.status === 401) { - error.alertAttrs.children = app.translator.trans('core.forum.change_email.incorrect_password_message'); + error.alert.children = app.translator.trans('core.forum.change_email.incorrect_password_message'); } super.onerror(error); diff --git a/js/src/forum/components/ForgotPasswordModal.js b/js/src/forum/components/ForgotPasswordModal.js index 22073709f7..7ccdcfd1a8 100644 --- a/js/src/forum/components/ForgotPasswordModal.js +++ b/js/src/forum/components/ForgotPasswordModal.js @@ -104,7 +104,7 @@ export default class ForgotPasswordModal extends Modal { onerror(error) { if (error.status === 404) { - error.alertAttrs.children = app.translator.trans('core.forum.forgot_password.not_found_message'); + error.alert.children = app.translator.trans('core.forum.forgot_password.not_found_message'); } super.onerror(error); diff --git a/js/src/forum/components/LogInModal.js b/js/src/forum/components/LogInModal.js index 716798f3ef..cd029f30d1 100644 --- a/js/src/forum/components/LogInModal.js +++ b/js/src/forum/components/LogInModal.js @@ -179,7 +179,7 @@ export default class LogInModal extends Modal { onerror(error) { if (error.status === 401) { - error.alertAttrs.children = app.translator.trans('core.forum.log_in.invalid_login_message'); + error.alert.children = app.translator.trans('core.forum.log_in.invalid_login_message'); } super.onerror(error); From 5e784787f8f0e8de460f44bdcc403a5bc7ada7d4 Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Tue, 30 Jun 2020 17:44:57 -0400 Subject: [PATCH 20/24] Rename alert "type" to "componentClass" --- js/src/common/components/AlertManager.js | 2 +- js/src/common/states/AlertManagerState.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/js/src/common/components/AlertManager.js b/js/src/common/components/AlertManager.js index b33b6c01a9..614e01b576 100644 --- a/js/src/common/components/AlertManager.js +++ b/js/src/common/components/AlertManager.js @@ -15,7 +15,7 @@ export default class AlertManager extends Component {
{Object.entries(this.state.getActiveAlerts()).map(([key, alert]) => (
- {(alert.type || Alert).component({ ...alert.attrs, ondismiss: this.state.dismiss.bind(this.state, key) })} + {(alert.componentClass || Alert).component({ ...alert.attrs, ondismiss: this.state.dismiss.bind(this.state, key) })}
))}
diff --git a/js/src/common/states/AlertManagerState.js b/js/src/common/states/AlertManagerState.js index a0d701e997..2443d10077 100644 --- a/js/src/common/states/AlertManagerState.js +++ b/js/src/common/states/AlertManagerState.js @@ -11,8 +11,8 @@ export default class AlertManagerState { /** * Show an Alert in the alerts area. */ - show(attrs, type) { - this.activeAlerts[++this.alertId] = { type, attrs }; + show(attrs, componentClass) { + this.activeAlerts[++this.alertId] = { componentClass, attrs }; m.redraw(); return this.alertId; From a61f8a8003158227cb81a41d77ee22994d9de0cc Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Tue, 30 Jun 2020 17:50:20 -0400 Subject: [PATCH 21/24] Add safety check for improper stuff being passed to alerts --- js/src/common/states/AlertManagerState.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/js/src/common/states/AlertManagerState.js b/js/src/common/states/AlertManagerState.js index 2443d10077..2fade58167 100644 --- a/js/src/common/states/AlertManagerState.js +++ b/js/src/common/states/AlertManagerState.js @@ -1,3 +1,5 @@ +import Alert from "../components/Alert"; + export default class AlertManagerState { constructor() { this.activeAlerts = {}; @@ -11,7 +13,15 @@ export default class AlertManagerState { /** * Show an Alert in the alerts area. */ - show(attrs, componentClass) { + show(attrs, componentClass=Alert) { + // Breaking Change Compliance Warning, Remove in Beta 15 + if (!(componentClass === Alert || componentClass.prototype instanceof Alert)) { + throw new Error('The AlertManager can only show Alerts'); + } + if (componentClass.init) { + throw new Error('The type parameter must be an alert class, not an alert instance'); + } + // End Change Compliance Warning, Remove in Beta 15 this.activeAlerts[++this.alertId] = { componentClass, attrs }; m.redraw(); From 7638478aba8c8a103722149cf242b353d77d734f Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Tue, 30 Jun 2020 18:02:14 -0400 Subject: [PATCH 22/24] Apply alert BC compliance warning to first argument --- js/src/common/states/AlertManagerState.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/js/src/common/states/AlertManagerState.js b/js/src/common/states/AlertManagerState.js index 2fade58167..8d3116c259 100644 --- a/js/src/common/states/AlertManagerState.js +++ b/js/src/common/states/AlertManagerState.js @@ -14,12 +14,12 @@ export default class AlertManagerState { * Show an Alert in the alerts area. */ show(attrs, componentClass=Alert) { - // Breaking Change Compliance Warning, Remove in Beta 15 - if (!(componentClass === Alert || componentClass.prototype instanceof Alert)) { - throw new Error('The AlertManager can only show Alerts'); - } - if (componentClass.init) { - throw new Error('The type parameter must be an alert class, not an alert instance'); + // Breaking Change Compliance Warning, Remove in Beta 15. + // This is applied to the first argument (attrs) because previously, the alert was passed as the first argument. + if (attrs === Alert || attrs instanceof Alert) { + // This is duplicated so that if the error is caught, an error message still shows up in the debug console. + console.error('The AlertManager can only show Alerts. Whichever extension triggered this alert should be updated to comply with beta 14.'); + throw new Error('The AlertManager can only show Alerts. Whichever extension triggered this alert should be updated to comply with beta 14.'); } // End Change Compliance Warning, Remove in Beta 15 this.activeAlerts[++this.alertId] = { componentClass, attrs }; From fb9676cdea7a20fd7aa6b1ea2a941ce96acb825b Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Tue, 30 Jun 2020 18:02:46 -0400 Subject: [PATCH 23/24] Prettify --- js/src/common/states/AlertManagerState.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/src/common/states/AlertManagerState.js b/js/src/common/states/AlertManagerState.js index 8d3116c259..76040fa235 100644 --- a/js/src/common/states/AlertManagerState.js +++ b/js/src/common/states/AlertManagerState.js @@ -1,4 +1,4 @@ -import Alert from "../components/Alert"; +import Alert from '../components/Alert'; export default class AlertManagerState { constructor() { @@ -13,7 +13,7 @@ export default class AlertManagerState { /** * Show an Alert in the alerts area. */ - show(attrs, componentClass=Alert) { + show(attrs, componentClass = Alert) { // Breaking Change Compliance Warning, Remove in Beta 15. // This is applied to the first argument (attrs) because previously, the alert was passed as the first argument. if (attrs === Alert || attrs instanceof Alert) { From 8653bf5890bedc4fb02327ab2d9e0c42c62612fe Mon Sep 17 00:00:00 2001 From: Alexander Skvortsov Date: Tue, 30 Jun 2020 18:04:22 -0400 Subject: [PATCH 24/24] Reverse order of arguments in alert POJO, as order doesn't matter, and this way it's more consistent --- js/src/common/states/AlertManagerState.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/common/states/AlertManagerState.js b/js/src/common/states/AlertManagerState.js index 76040fa235..6dd2a0f298 100644 --- a/js/src/common/states/AlertManagerState.js +++ b/js/src/common/states/AlertManagerState.js @@ -22,7 +22,7 @@ export default class AlertManagerState { throw new Error('The AlertManager can only show Alerts. Whichever extension triggered this alert should be updated to comply with beta 14.'); } // End Change Compliance Warning, Remove in Beta 15 - this.activeAlerts[++this.alertId] = { componentClass, attrs }; + this.activeAlerts[++this.alertId] = { attrs, componentClass }; m.redraw(); return this.alertId;