From 0442a3115395f8949080c5cdd2ec6bfc04fb8b50 Mon Sep 17 00:00:00 2001 From: Alex M Date: Sun, 2 Oct 2016 20:40:53 +0300 Subject: [PATCH] [Notification] Rename variables for clarity In the spec: NotificationService knows about concepts like "info", "alert" and "error" only, having a plain info notification model called successModel doesn't help much and may be confusing at times (makes you think it's something the service treats separately). --- .../notification/src/NotificationService.js | 18 +++---- .../test/NotificationServiceSpec.js | 50 ++++++++++--------- 2 files changed, 35 insertions(+), 33 deletions(-) diff --git a/platform/commonUI/notification/src/NotificationService.js b/platform/commonUI/notification/src/NotificationService.js index e37571759a..8f5b71ec42 100644 --- a/platform/commonUI/notification/src/NotificationService.js +++ b/platform/commonUI/notification/src/NotificationService.js @@ -109,18 +109,18 @@ define( * @memberof platform/commonUI/notification * @constructor * @param $timeout the Angular $timeout service - * @param DEFAULT_AUTO_DISMISS The period of time that an + * @param defaultAutoDismissTimeout The period of time that an * auto-dismissed message will be displayed for. - * @param MINIMIZE_TIMEOUT When notifications are minimized, a brief + * @param minimizeAnimationTimeout When notifications are minimized, a brief * animation is shown. This animation requires some time to execute, * so a timeout is required before the notification is hidden */ - function NotificationService($timeout, topic, DEFAULT_AUTO_DISMISS, MINIMIZE_TIMEOUT) { + function NotificationService($timeout, topic, defaultAutoDismissTimeout, minimizeAnimationTimeout) { this.notifications = []; this.$timeout = $timeout; this.highest = { severity: "info" }; - this.DEFAULT_AUTO_DISMISS = DEFAULT_AUTO_DISMISS; - this.MINIMIZE_TIMEOUT = MINIMIZE_TIMEOUT; + this.AUTO_DISMISS_TIMEOUT = defaultAutoDismissTimeout; + this.MINIMIZE_ANIMATION_TIMEOUT = minimizeAnimationTimeout; this.topic = topic; /* @@ -162,7 +162,7 @@ define( // in order to allow the minimize animation to run through. service.$timeout(function () { service.setActiveNotification(service.selectNextNotification()); - }, service.MINIMIZE_TIMEOUT); + }, service.MINIMIZE_ANIMATION_TIMEOUT); } }; @@ -325,7 +325,7 @@ define( notificationModel.severity = notificationModel.severity || "info"; if (notificationModel.autoDismiss === true) { - notificationModel.autoDismiss = this.DEFAULT_AUTO_DISMISS; + notificationModel.autoDismiss = this.AUTO_DISMISS_TIMEOUT; } //Notifications support a 'dismissable' attribute. This is a @@ -366,7 +366,7 @@ define( */ this.active.timeout = this.$timeout(function () { activeNotification.dismissOrMinimize(); - }, this.DEFAULT_AUTO_DISMISS); + }, this.AUTO_DISMISS_TIMEOUT); } return notification; @@ -390,7 +390,7 @@ define( if (notification && (notification.model.autoDismiss || this.selectNextNotification())) { - timeout = notification.model.autoDismiss || this.DEFAULT_AUTO_DISMISS; + timeout = notification.model.autoDismiss || this.AUTO_DISMISS_TIMEOUT; this.active.timeout = this.$timeout(function () { notification.dismissOrMinimize(); }, timeout); diff --git a/platform/commonUI/notification/test/NotificationServiceSpec.js b/platform/commonUI/notification/test/NotificationServiceSpec.js index b74b9e299a..715894a077 100644 --- a/platform/commonUI/notification/test/NotificationServiceSpec.js +++ b/platform/commonUI/notification/test/NotificationServiceSpec.js @@ -30,9 +30,9 @@ define( mockTimeout, mockAutoDismiss, mockMinimizeTimeout, - successModel, mockTopicFunction, mockTopicObject, + infoModel, errorModel; beforeEach(function () { @@ -44,27 +44,29 @@ define( mockAutoDismiss = mockMinimizeTimeout = 1000; notificationService = new NotificationService( mockTimeout, mockTopicFunction, mockAutoDismiss, mockMinimizeTimeout); - successModel = { - title: "Mock Success Notification", + + infoModel = { + title: "Mock Info Notification", severity: "info" }; + errorModel = { title: "Mock Error Notification", severity: "error" }; }); - it("activates success notifications", function () { + it("activates info notifications", function () { var activeNotification; - notificationService.notify(successModel); + notificationService.notify(infoModel); activeNotification = notificationService.getActiveNotification(); - expect(activeNotification.model).toBe(successModel); + expect(activeNotification.model).toBe(infoModel); }); it("notifies listeners on dismissal of notification", function () { var notification, dismissListener = jasmine.createSpy("ondismiss"); - notification = notificationService.notify(successModel); + notification = notificationService.notify(infoModel); notification.onDismiss(dismissListener); expect(mockTopicObject.listen).toHaveBeenCalled(); notification.dismiss(); @@ -83,12 +85,12 @@ define( expect(activeNotification.model.severity).toBe("info"); }); - it("gets a new success notification with numerical auto-dismiss specified. ", function () { + it("gets a new info notification with numerical auto-dismiss specified. ", function () { var activeNotification; - successModel.autoDismiss = 1000; - notificationService.notify(successModel); + infoModel.autoDismiss = 1000; + notificationService.notify(infoModel); activeNotification = notificationService.getActiveNotification(); - expect(activeNotification.model).toBe(successModel); + expect(activeNotification.model).toBe(infoModel); mockTimeout.mostRecentCall.args[0](); expect(mockTimeout.calls.length).toBe(2); mockTimeout.mostRecentCall.args[0](); @@ -98,10 +100,10 @@ define( it("gets a new notification with boolean auto-dismiss specified. ", function () { var activeNotification; - successModel.autoDismiss = true; - notificationService.notify(successModel); + infoModel.autoDismiss = true; + notificationService.notify(infoModel); activeNotification = notificationService.getActiveNotification(); - expect(activeNotification.model).toBe(successModel); + expect(activeNotification.model).toBe(infoModel); mockTimeout.mostRecentCall.args[0](); expect(mockTimeout.calls.length).toBe(2); mockTimeout.mostRecentCall.args[0](); @@ -113,11 +115,11 @@ define( var notification, activeNotification; - successModel.autoDismiss = false; - notification = notificationService.notify(successModel); + infoModel.autoDismiss = false; + notification = notificationService.notify(infoModel); activeNotification = notificationService.getActiveNotification(); - expect(activeNotification.model).toBe(successModel); + expect(activeNotification.model).toBe(infoModel); notification.minimize(); mockTimeout.mostRecentCall.args[0](); activeNotification = notificationService.getActiveNotification(); @@ -129,11 +131,11 @@ define( var notification, activeNotification; - successModel.autoDismiss = false; - notification = notificationService.notify(successModel); + infoModel.autoDismiss = false; + notification = notificationService.notify(infoModel); activeNotification = notificationService.getActiveNotification(); - expect(activeNotification.model).toBe(successModel); + expect(activeNotification.model).toBe(infoModel); notification.dismiss(); activeNotification = notificationService.getActiveNotification(); expect(activeNotification).toBeUndefined(); @@ -144,11 +146,11 @@ define( it("auto-dismisses the previously active notification, making the new notification active", function () { var activeNotification; //First pre-load with a info message - notificationService.notify(successModel); + notificationService.notify(infoModel); activeNotification = notificationService.getActiveNotification(); //Initially expect the active notification to be info - expect(activeNotification.model).toBe(successModel); + expect(activeNotification.model).toBe(infoModel); //Then notify of an error notificationService.notify(errorModel); //But it should be auto-dismissed and replaced with the @@ -167,7 +169,7 @@ define( //First pre-load with an error message notificationService.notify(errorModel); //Then notify of info - notificationService.notify(successModel); + notificationService.notify(infoModel); expect(notificationService.notifications.length).toEqual(2); //Mock the auto-minimize mockTimeout.mostRecentCall.args[0](); @@ -180,7 +182,7 @@ define( expect(notificationService.notifications.length).toEqual(2); activeNotification = notificationService.getActiveNotification(); - expect(activeNotification.model).toBe(successModel); + expect(activeNotification.model).toBe(infoModel); expect(errorModel.minimized).toEqual(true); });