Show multiple alerts in a stack and allow for custom alert icons. #3209

evhan55 · 2018-09-22T21:48:11Z

Resolves

This is progress towards: #2956.

Proposed Changes

Displays stacks of alerts vertically.
Shows an icon in an alert if one is provided.

Reason for Changes

To improve on previous alerts that displayed directly on top of each other, and to add further customizability with optional icons.

Notes

Is a prerequisite for this PR in scratch-vm: Show customized alerts on hardware extension peripheral errors. scratch-vm#1607.

src/components/alerts/alert.jsx

+            'handleOnCloseAlert'
+        ]);
+    }
+    handleOnCloseAlert () {


src/components/alerts/alert.jsx

+            'handleOnCloseAlert'
+        ]);
+    }
+    handleOnCloseAlert () {


src/reducers/alerts.js

+        if (extensionId) { // if it's an extension
+            const extension = extensionData.find(ext => ext.extensionId === extensionId);
+            if (extension && extension.name) {
+                // TODO: is this the right place to assemble this message?


src/reducers/alerts.js

 };

-const showAlert = function (message) {
+const showAlert = function (data) {


src/reducers/alerts.js


-const closeAlert = function () {
-    return {type: CLOSE_ALERT};
+const closeAlert = function (index) {


…ely.

…ssage construction to reducer.

…andle index correctly. Removed AlertsComponent and added new AlertComponent.

rschamp

Looks good to me @evhan55! I have a few comments that could be addressed in the follow-up PR

src/containers/alert.jsx

+            <AlertComponent
+                iconURL={this.props.iconURL}
+                message={this.props.message}
+                onCloseAlert={this.handleOnCloseAlert}


src/containers/alerts.jsx

+    className,
+    onCloseAlert
+}) => (
+    <Box


src/reducers/alerts.js

+/**
+ * Function to show an alert with the given input data.
+ *
+ * @param {object} data - data with the following props: {message, extensionId=null}


ericrosenbaum

👍

evhan55 commented Sep 22, 2018

View reviewed changes

src/components/alerts/alert.jsx Outdated

'handleOnCloseAlert'

]);

}

handleOnCloseAlert () {

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

evhan55 added the pr - needs review label Sep 22, 2018

evhan55 requested a review from rschamp September 22, 2018 21:51

evhan55 assigned rschamp Sep 22, 2018

evhan55 mentioned this pull request Sep 22, 2018

Show customized alerts on hardware extension peripheral errors. scratchfoundation/scratch-vm#1607

Merged

thisandagain added this to the September 2018 milestone Sep 24, 2018

rschamp suggested changes Sep 24, 2018

View reviewed changes

rschamp added pr - changes requested and removed pr - needs review labels Sep 24, 2018

rschamp assigned evhan55 and unassigned rschamp Sep 24, 2018

evhan55 added 8 commits September 24, 2018 18:43

Beginning multiple alerts component.

0e5fa0b

Adding a custom extension image to alerts, etc.

5e6db18

Customizing alerts for extensions.

abe33f1

Adding Alerts container to handle click, and pass key prop appropriat…

bdaa407

…ely.

Moved alert icon check/display up to reducer, and also moved alert me…

d5eb947

…ssage construction to reducer.

Moving click handling to alert component from container in order to h…

63c3157

…andle index correctly. Removed AlertsComponent and added new AlertComponent.

Adding Alert container to hold index and click handler.

21a9afb

Adding jsdocs to the alert reducers.

0265a86

evhan55 mentioned this pull request Sep 24, 2018

Alerts for hardware and other info: overall design #2956

Closed

evhan55 removed their assignment Sep 24, 2018

evhan55 added pr - needs review and removed pr - changes requested labels Sep 24, 2018

evhan55 assigned rschamp Sep 24, 2018

rschamp previously approved these changes Sep 25, 2018

View reviewed changes

rschamp added pr - good to merge and removed pr - needs review labels Sep 25, 2018

rschamp assigned evhan55 and unassigned rschamp Sep 25, 2018

Adding package.json.

1d68965

evhan55 dismissed rschamp’s stale review via 1d68965 September 25, 2018 20:04

evhan55 requested a review from ericrosenbaum September 26, 2018 15:37

Merge branch 'develop' into multiple-alerts

18e35ef

ericrosenbaum approved these changes Sep 26, 2018

View reviewed changes

ericrosenbaum merged commit ca51b63 into scratchfoundation:develop Sep 26, 2018

evhan55 mentioned this pull request Oct 1, 2018

Alerts API for different types of alerts #3265

Closed

evhan55 deleted the multiple-alerts branch October 10, 2018 20:36

Show multiple alerts in a stack and allow for custom alert icons. #3209

Show multiple alerts in a stack and allow for custom alert icons. #3209

Uh oh!

Conversation

evhan55 commented Sep 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Resolves

Proposed Changes

Reason for Changes

Notes

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

rschamp left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

This comment was marked as abuse.

Uh oh!

ericrosenbaum left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

evhan55 commented Sep 22, 2018 •

edited

Loading