From f42ba8349b8adb05aaaa5230bdf8353a8a5c5a15 Mon Sep 17 00:00:00 2001 From: Anton Antonov Date: Sat, 18 Mar 2017 18:55:28 +0200 Subject: [PATCH 1/9] Extract `getNotification` from `removeNotification` And optimize `getNotification` to stop iterating once a notification matching the given `uid` is found. --- src/NotificationSystem.jsx | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/NotificationSystem.jsx b/src/NotificationSystem.jsx index adbef3e..777f3ea 100644 --- a/src/NotificationSystem.jsx +++ b/src/NotificationSystem.jsx @@ -152,18 +152,30 @@ var NotificationSystem = React.createClass({ return _notification; }, - removeNotification: function(notification) { + getNotification: function(notification) { var self = this; + var foundNotification = null; + Object.keys(this.refs).forEach(function(container) { if (container.indexOf('container') > -1) { Object.keys(self.refs[container].refs).forEach(function(_notification) { var uid = notification.uid ? notification.uid : notification; if (_notification === 'notification-' + uid) { - self.refs[container].refs[_notification]._hideNotification(); + // NOTE: Stop iterating further and return the found notification. + // Since UIDs are uniques and there won't be another notification found. + foundNotification = self.refs[container].refs[_notification]; + return } }); } }); + + return foundNotification; + }, + + removeNotification: function(notification) { + var foundNotification = this.getNotification(notification); + foundNotification && foundNotification._hideNotification(); }, clearNotifications: function() { @@ -221,7 +233,6 @@ var NotificationSystem = React.createClass({
{ containers }
- ); } }); From b84d11ea7463354cf71e514484e0d32cf7cf7af4 Mon Sep 17 00:00:00 2001 From: Anton Antonov Date: Sat, 18 Mar 2017 21:54:32 +0200 Subject: [PATCH 2/9] [#33] Add notification editing --- README.md | 6 +++++ src/NotificationSystem.jsx | 41 ++++++++++++++++++++++++++++---- test/notification-system.test.js | 32 +++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index f8dc04b..6677d0b 100644 --- a/README.md +++ b/README.md @@ -86,6 +86,12 @@ Returns the notification object to be used to programmatically dismiss a notific Remove a notification programmatically. You can pass an object returned by `addNotification()` or by `onAdd()` callback. If passing an object, you need to make sure it must contain the `uid` property. You can pass only the `uid` too: `removeNotification(uid)`. + +### `editNotification(notification)` + +Edit a notification programmatically. You can pass an object previously returned by `addNotification()` or by `onAdd()` callback. If passing an object, you need to make sure it must contain the `uid` property. You can pass only the `uid` too: `editNotification(uid)`. + + ### `clearNotifications()` Removes ALL notifications programatically. diff --git a/src/NotificationSystem.jsx b/src/NotificationSystem.jsx index 777f3ea..d1d460f 100644 --- a/src/NotificationSystem.jsx +++ b/src/NotificationSystem.jsx @@ -152,7 +152,7 @@ var NotificationSystem = React.createClass({ return _notification; }, - getNotification: function(notification) { + getNotificationRef: function(notification) { var self = this; var foundNotification = null; @@ -164,7 +164,7 @@ var NotificationSystem = React.createClass({ // NOTE: Stop iterating further and return the found notification. // Since UIDs are uniques and there won't be another notification found. foundNotification = self.refs[container].refs[_notification]; - return + return; } }); } @@ -174,8 +174,41 @@ var NotificationSystem = React.createClass({ }, removeNotification: function(notification) { - var foundNotification = this.getNotification(notification); - foundNotification && foundNotification._hideNotification(); + var foundNotification = this.getNotificationRef(notification); + return foundNotification && foundNotification._hideNotification(); + }, + + editNotification: function(notification, newNotification) { + var foundNotification = null; + // NOTE: Find state notification to update by using + // `setState` and forcing React to re-render the component. + var uid = notification.uid ? notification.uid : notification; + + var newNotifications = this.state.notifications.filter(function(stateNotification) { + if (uid === stateNotification.uid) { + foundNotification = stateNotification; + return false; + } + + return true; + }); + + + if (!foundNotification) { + return; + } + + newNotifications.push( + merge( + {}, + foundNotification, + newNotification + ) + ); + + this.setState({ + notifications: newNotifications + }); }, clearNotifications: function() { diff --git a/test/notification-system.test.js b/test/notification-system.test.js index f2a5564..e8139bd 100644 --- a/test/notification-system.test.js +++ b/test/notification-system.test.js @@ -172,6 +172,38 @@ describe('Notification Component', function() { done(); }); + it('should edit an existing notification using returned object', (done) => { + const notificationCreated = component.addNotification(defaultNotification); + const notification = TestUtils.scryRenderedDOMComponentsWithClass(instance, 'notification'); + expect(notification.length).toEqual(1); + + const newTitle = 'foo'; + const newContent = 'foobar'; + + component.editNotification(notificationCreated, { title: newTitle, message: newContent }); + clock.tick(1000); + const notificationEdited = TestUtils.findRenderedDOMComponentWithClass(instance, 'notification'); + expect(notificationEdited.getElementsByClassName('notification-title')[0].textContent).toEqual(newTitle); + expect(notificationEdited.getElementsByClassName('notification-message')[0].textContent).toEqual(newContent); + done(); + }); + + it('should edit an existing notification using uid', (done) => { + const notificationCreated = component.addNotification(defaultNotification); + const notification = TestUtils.scryRenderedDOMComponentsWithClass(instance, 'notification'); + expect(notification.length).toEqual(1); + + const newTitle = 'foo'; + const newContent = 'foobar'; + + component.editNotification(notificationCreated.uid, { title: newTitle, message: newContent }); + clock.tick(1000); + const notificationEdited = TestUtils.findRenderedDOMComponentWithClass(instance, 'notification'); + expect(notificationEdited.getElementsByClassName('notification-title')[0].textContent).toEqual(newTitle); + expect(notificationEdited.getElementsByClassName('notification-message')[0].textContent).toEqual(newContent); + done(); + }); + it('should remove all notifications', done => { component.addNotification(defaultNotification); component.addNotification(defaultNotification); From 094e00ad348525b2400b0a6135d3c9d4a26656e1 Mon Sep 17 00:00:00 2001 From: Anton Antonov Date: Sun, 19 Mar 2017 18:58:28 +0200 Subject: [PATCH 3/9] Optimize NotificationSystem._didNotificationRemoved No need to perform a check again single we already know that it's a match when we assign `notification`. --- src/NotificationSystem.jsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/NotificationSystem.jsx b/src/NotificationSystem.jsx index d1d460f..c91e3a9 100644 --- a/src/NotificationSystem.jsx +++ b/src/NotificationSystem.jsx @@ -66,8 +66,9 @@ var NotificationSystem = React.createClass({ var notifications = this.state.notifications.filter(function(toCheck) { if (toCheck.uid === uid) { notification = toCheck; + return false; } - return toCheck.uid !== uid; + return true; }); if (notification && notification.onRemove) { From a3f8e09e0f97ae55abfc74e7e817d9a1f8087162 Mon Sep 17 00:00:00 2001 From: Andrey Date: Mon, 10 Apr 2017 10:04:02 +0200 Subject: [PATCH 4/9] Added prop-types package dependency, updated all compomponents to use prop-types lib instead of React.PropTypes, added create-react-class package dependency, updated all components to use it instead of React.createClasss --- example/src/scripts/App.jsx | 3 ++- example/src/scripts/CustomElement.jsx | 3 ++- example/src/scripts/NotificationGenerator.jsx | 8 +++++--- package.json | 4 +++- src/NotificationContainer.jsx | 10 ++++++---- src/NotificationItem.jsx | 20 ++++++++++--------- src/NotificationSystem.jsx | 14 +++++++------ 7 files changed, 37 insertions(+), 25 deletions(-) diff --git a/example/src/scripts/App.jsx b/example/src/scripts/App.jsx index 429abfb..0615311 100644 --- a/example/src/scripts/App.jsx +++ b/example/src/scripts/App.jsx @@ -1,5 +1,6 @@ var React = require('react'); var ReactDOM = require('react-dom'); +var createReactClass = require('create-react-class'); var NotificationSystem = require('NotificationSystem'); var constants = require('constants'); var NotificationGenerator = require('./NotificationGenerator'); @@ -14,7 +15,7 @@ var _getRandomPosition = function() { // Styles require('styles/base'); -NotificationSystemExample = React.createClass({ +NotificationSystemExample = createReactClass({ displayName: 'App', diff --git a/example/src/scripts/CustomElement.jsx b/example/src/scripts/CustomElement.jsx index 6889204..751bd51 100644 --- a/example/src/scripts/CustomElement.jsx +++ b/example/src/scripts/CustomElement.jsx @@ -1,4 +1,5 @@ var React = require('react'); +var PropTypes = require('prop-types'); function buttonClicked() { alert('I\'m a custom button inside a custom element that was clicked'); @@ -14,7 +15,7 @@ function CustomElement(props) { } CustomElement.propTypes = { - name: React.PropTypes.string + name: PropTypes.string }; module.exports = CustomElement; diff --git a/example/src/scripts/NotificationGenerator.jsx b/example/src/scripts/NotificationGenerator.jsx index 57fa4bb..6eadf46 100644 --- a/example/src/scripts/NotificationGenerator.jsx +++ b/example/src/scripts/NotificationGenerator.jsx @@ -1,9 +1,11 @@ var React = require('react'); +var createReactClass = require('create-react-class'); +var PropTypes = require('prop-types'); // Styles require('styles/generator'); -module.exports = React.createClass({ +module.exports = createReactClass({ displayName: 'NotificationGenerator', @@ -134,8 +136,8 @@ module.exports = React.createClass({ }, propTypes: { - notifications: React.PropTypes.func.isRequired, - allowHTML: React.PropTypes.func + notifications: PropTypes.func.isRequired, + allowHTML: PropTypes.func }, render: function() { diff --git a/package.json b/package.json index 54f326b..f98d152 100644 --- a/package.json +++ b/package.json @@ -32,7 +32,9 @@ }, "homepage": "https://github.com/igorprado/react-notification-system", "dependencies": { - "object-assign": "^4.0.1" + "create-react-class": "^15.5.1", + "object-assign": "^4.0.1", + "prop-types": "^15.5.6" }, "peerDependencies": { "react": "0.14.x || ^15.0.0", diff --git a/src/NotificationContainer.jsx b/src/NotificationContainer.jsx index 2c1dbec..0b6e044 100644 --- a/src/NotificationContainer.jsx +++ b/src/NotificationContainer.jsx @@ -1,13 +1,15 @@ var React = require('react'); +var createReactClass = require('create-react-class'); +var PropTypes = require('prop-types'); var NotificationItem = require('./NotificationItem'); var Constants = require('./constants'); -var NotificationContainer = React.createClass({ +var NotificationContainer = createReactClass({ propTypes: { - position: React.PropTypes.string.isRequired, - notifications: React.PropTypes.array.isRequired, - getStyles: React.PropTypes.object + position: PropTypes.string.isRequired, + notifications: PropTypes.array.isRequired, + getStyles: PropTypes.object }, _style: {}, diff --git a/src/NotificationItem.jsx b/src/NotificationItem.jsx index a8e1fbb..7c015b4 100644 --- a/src/NotificationItem.jsx +++ b/src/NotificationItem.jsx @@ -1,4 +1,6 @@ var React = require('react'); +var createReactClass = require('create-react-class'); +var PropTypes = require('prop-types'); var ReactDOM = require('react-dom'); var Constants = require('./constants'); var Helpers = require('./helpers'); @@ -24,17 +26,17 @@ var whichTransitionEvent = function() { return transition; }; -var NotificationItem = React.createClass({ +var NotificationItem = createReactClass({ propTypes: { - notification: React.PropTypes.object, - getStyles: React.PropTypes.object, - onRemove: React.PropTypes.func, - allowHTML: React.PropTypes.bool, - noAnimation: React.PropTypes.bool, - children: React.PropTypes.oneOfType([ - React.PropTypes.string, - React.PropTypes.element + notification: PropTypes.object, + getStyles: PropTypes.object, + onRemove: PropTypes.func, + allowHTML: PropTypes.bool, + noAnimation: PropTypes.bool, + children: PropTypes.oneOfType([ + PropTypes.string, + PropTypes.element ]) }, diff --git a/src/NotificationSystem.jsx b/src/NotificationSystem.jsx index adbef3e..a1a7d16 100644 --- a/src/NotificationSystem.jsx +++ b/src/NotificationSystem.jsx @@ -1,10 +1,12 @@ var React = require('react'); +var createReactClass = require('create-react-class'); +var PropTypes = require('prop-types'); var merge = require('object-assign'); var NotificationContainer = require('./NotificationContainer'); var Constants = require('./constants'); var Styles = require('./styles'); -var NotificationSystem = React.createClass({ +var NotificationSystem = createReactClass({ uid: 3400, @@ -86,12 +88,12 @@ var NotificationSystem = React.createClass({ }, propTypes: { - style: React.PropTypes.oneOfType([ - React.PropTypes.bool, - React.PropTypes.object + style: PropTypes.oneOfType([ + PropTypes.bool, + PropTypes.object ]), - noAnimation: React.PropTypes.bool, - allowHTML: React.PropTypes.bool + noAnimation: PropTypes.bool, + allowHTML: PropTypes.bool }, getDefaultProps: function() { From c9f266266ccf345844d05a5cc44e4ceb340a85ec Mon Sep 17 00:00:00 2001 From: Jack Willis-Craig Date: Mon, 24 Apr 2017 23:22:47 +1000 Subject: [PATCH 5/9] fix: remove notification from state before calling onRemove --- src/NotificationSystem.jsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/NotificationSystem.jsx b/src/NotificationSystem.jsx index adbef3e..4c47c1c 100644 --- a/src/NotificationSystem.jsx +++ b/src/NotificationSystem.jsx @@ -70,13 +70,13 @@ var NotificationSystem = React.createClass({ return toCheck.uid !== uid; }); - if (notification && notification.onRemove) { - notification.onRemove(notification); - } - if (this._isMounted) { this.setState({ notifications: notifications }); } + + if (notification && notification.onRemove) { + notification.onRemove(notification); + } }, getInitialState: function() { From ceaf62f06b2809b653f8bb0c54faef5ae6e57b0a Mon Sep 17 00:00:00 2001 From: Igor Prado Date: Wed, 3 May 2017 15:26:45 -0500 Subject: [PATCH 6/9] Bumped package version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f98d152..31cc5a6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "react-notification-system", - "version": "0.2.13", + "version": "0.2.14", "description": "A React Notification System fully customized", "main": "dist/NotificationSystem.js", "scripts": { From 63c5bfd06274cb3f8f62b4e084dd69ede8f2c370 Mon Sep 17 00:00:00 2001 From: Igor Prado Date: Wed, 3 May 2017 15:31:26 -0500 Subject: [PATCH 7/9] Added edit notification changes to CHANGELOG --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18f9e48..d35a195 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # CHANGELOG +## 0.2.14 - May 3, 2017 + +* Ability to [edit notifications](https://github.com/igorprado/react-notification-system#removenotificationnotification) (thanks to @syndbg) +* Removed deprecation warning. Now using `prop-types` and `create-react-class`packages. (thanks to @andrewBalekha) + ## 0.2.13 - Mar 14, 2017 * UMD support. (thanks to @jochenberger) From 21de0b8debe090527ba0e10c914895b38e62cd7f Mon Sep 17 00:00:00 2001 From: Igor Prado Date: Wed, 3 May 2017 15:33:34 -0500 Subject: [PATCH 8/9] Added onRemove fix to CHANGELOG --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d35a195..a994f53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,9 @@ ## 0.2.14 - May 3, 2017 -* Ability to [edit notifications](https://github.com/igorprado/react-notification-system#removenotificationnotification) (thanks to @syndbg) +* Ability to [edit notifications](https://github.com/igorprado/react-notification-system#removenotificationnotification). (thanks to @syndbg) * Removed deprecation warning. Now using `prop-types` and `create-react-class`packages. (thanks to @andrewBalekha) +* Fix calling `onRemove` before updating the notifications state. (thanks to @szdc) ## 0.2.13 - Mar 14, 2017 From 3d6e955ae54c463febb360ca42a2e9c9cc63f8ce Mon Sep 17 00:00:00 2001 From: Igor Prado Date: Wed, 3 May 2017 15:39:24 -0500 Subject: [PATCH 9/9] Disabled react/display-name rule for now --- .eslintrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.eslintrc b/.eslintrc index 6b0a502..1d72f1b 100644 --- a/.eslintrc +++ b/.eslintrc @@ -17,7 +17,7 @@ } }, "rules": { - "react/display-name": 2, + "react/display-name": 0, "react/jsx-curly-spacing": [2, "always"], "react/jsx-no-duplicate-props": 2, "react/jsx-no-undef": 2,