Update: JSDoc documentation for notification system (fixes #810)

js/collections/notifyPushCollection.js

@@ -1,8 +1,59 @@
+/**
+ * @file Push Notification Collection - Manages queue of push notifications
+ * @module core/js/collections/notifyPushCollection
+ *
+ * Collection managing the queue and display of push notifications.
+ * Ensures maximum of **2 push notifications** are visible simultaneously.
+ *
+ * **SINGLETON PATTERN**: Only ONE instance of NotifyPushCollection should exist
+ * in the application. Do NOT instantiate this class directly.
+ *
+ * **Side Effects (Important):**
+ * - Adding a model **automatically triggers display logic** (not just storage)
+ * - Collection **mutates models** by setting `_isActive: true` when displaying
+ * - Models with `_isActive: false` are queued; collection activates them when space available
+ * - **Creates {@link NotifyPushView} instances** automatically when displaying (side effect)
+ * - Listens to global `notify:pushRemoved` event to activate queued notifications
+ *
+ * **Queueing Behavior:**
+ * - Maximum 2 push notifications visible at once
+ * - Additional notifications queued until space available
+ * - Automatic positioning (top-right of viewport)
+ * - Delayed display via model `_delay` property
+ *
+ * **Dependencies:**
+ * - Requires {@link NotifyPushView} for rendering
+ * - Listens to {@link module:core/js/adapt Adapt} global event bus
+ * - Expects models to have `_isActive` and `_delay` properties
+ *
+ * **Known Issues & Improvements:**
+ * - **Issue:** Max queue size hardcoded to 2 - should be configurable
+ * - **Issue:** No queue overflow handling or limits
+ * - **Enhancement:** Add configurable `_maxVisible` option
+ * - **Enhancement:** Add `clearQueue()` method to dismiss all queued notifications
+ * - **Enhancement:** Return view instance from `showPush()` for external control
+ * - **Enhancement:** Handle race conditions when rapid add/remove occurs
+ *
+ * **Important:** Do NOT manually instantiate with `new NotifyPushCollection()`.
+ * The singleton instance is accessed internally via `notify.notifyPushes`.
+ * Developers should use `notify.push(options)` instead of `notify.notifyPushes.add(model)`.
+ *
+ * @see {@link NotifyPushView} for rendering implementation
+ */
+
 import Adapt from 'core/js/adapt';
 import NotifyPushView from 'core/js/views/notifyPushView';
 import NotifyModel from 'core/js/models/notifyModel';
 
-// Build a collection to store push notifications
+/**
+ * @class NotifyPushCollection
+ * @extends {Backbone.Collection}
+ * @singleton
+ *
+ * Only **one instance** should exist in the application.
+ * Do not use `new NotifyPushCollection()` directly.
+ * Access through Adapt's internal notification system.
+ */
 export default class NotifyPushCollection extends Backbone.Collection {
 
   initialize() {
@@ -21,11 +72,33 @@ export default class NotifyPushCollection extends Backbone.Collection {
     this.showPush(model);
   }
 
+  /**
+   * Determines if another push notification can be shown.
+   *
+   * **Logic:** Counts active notifications (where `_isActive === true`)
+   * and returns `true` if fewer than 2 are currently displayed.
+   *
+   * @returns {boolean} `true` if fewer than 2 active notifications, `false` otherwise
+   * @private
+   */
   canShowPush() {
     const availablePushNotifications = this.where({ _isActive: true });
     return (availablePushNotifications.length < 2);
   }
 
+  /**
+   * Displays a push notification by creating its view.
+   *
+   * **Side effect:** Creates new {@link NotifyPushView} instance.
+   *
+   * **Delay Behavior:**
+   * - If `model._delay` is set, waits that many milliseconds before showing
+   * - Useful for staggering multiple notifications
+   *
+   * @param {NotifyModel} model - The notification model to display
+   * @param {number} [model._delay=0] - Milliseconds to wait before showing
+   * @private
+   */
   showPush(model) {
     _.delay(() => {
       new NotifyPushView({

js/models/notifyModel.js

@@ -1,5 +1,39 @@
+/**
+ * @file Notification Model - Data model for notification instances
+ * @module core/js/models/notifyModel
+ * @description Model representing a single notification (popup, alert, prompt, or push).
+ * Manages notification state, timing, and lifecycle.
+ *
+ * **Side Effects:**
+ * - `close()` can only be called once (idempotent - subsequent calls ignored)
+ * - Triggers `'closed'` event when closed
+ * - Automatically sets `_hasClosed: true` on close
+ *
+ * **Important Properties:**
+ * - `_isActive` - Whether notification is currently displayed
+ * - `_timeout` - Auto-close delay in milliseconds (default: 3000)
+ * - `_delay` - Display delay before showing (default: 0)
+ * - `_hasClosed` - Tracks if notification already closed
+ *
+ * **Known Issues & Improvements:**
+ * - **Issue:** No validation - accepts any property without schema validation
+ * - **Issue:** Silent failures - invalid `_timeout` or `_delay` values don't throw errors
+ * - **Enhancement:** Add property validation for `_timeout`/`_delay` (must be numbers >= 0)
+ * - **Enhancement:** Add `isActive()` getter method instead of direct property access
+ * - **Enhancement:** Support cancellation tokens for `onClosed()` promise
+ *
+ * **Important:** Do NOT manually instantiate with `new NotifyModel()`.
+ * Models are created internally by notify service methods: `notify.push()`, `notify.popup()`,
+ * `notify.alert()`, and `notify.prompt()`.
+ */
+
 import LockingModel from 'core/js/models/lockingModel';
 
+/**
+ * @class NotifyModel
+ * @classdesc Lifecycle: Created → Rendered → Displayed → Closed → Removed
+ * @extends {module:core/js/models/lockingModel.LockingModel}
+ */
 export default class NotifyModel extends LockingModel {
 
   defaults() {
@@ -12,12 +46,34 @@ export default class NotifyModel extends LockingModel {
     };
   }
 
+  /**
+   * Closes the notification and triggers 'closed' event.
+   * **Idempotent:** Can be called multiple times safely (subsequent calls ignored).
+   * @fires closed
+   * @example
+   * notification.close();
+   */
   close() {
     if (this.get('_hasClosed')) return;
     this.set('_hasClosed', true);
     this.trigger('closed');
   }
 
+  /**
+   * Returns a promise that resolves when the notification is closed.
+   * Useful for waiting on notification completion before proceeding.
+   * @async
+   * @returns {Promise<void>} Resolves when notification closes
+   * @example
+   * await notification.onClosed();
+   * console.log('Notification closed');
+   *
+   * @example
+   * const notif1 = notify.popup({ title: 'First' });
+   * await notif1.onClosed();
+   * const notif2 = notify.popup({ title: 'Second' });
+   * await notif2.onClosed();
+   */
   async onClosed() {
     if (this.get('_hasClosed')) return;
     return new Promise(resolve => this.once('closed', resolve));

js/notify.js

@@ -1,4 +1,100 @@
+/**
+ * @file Notification Service - Global notification system singleton
+ * @module core/js/notify
+ * @description The Notification Service provides a unified API for displaying notifications,
+ * alerts, prompts, and push notifications throughout the framework. This singleton instance
+ * manages all notification types and their lifecycles.
+ *
+ * **Module Responsibility:** Centralized notification management for all user-facing messages
+ *
+ * **Public API for Plugins:**
+ *   - `notify.popup(options)` - Display modal popup
+ *   - `notify.alert(options)` - Display alert with confirm button
+ *   - `notify.prompt(options)` - Display prompt with custom buttons
+ *   - `notify.push(options)` - Display non-blocking push notification
+ *   - `notify.read(text)` - Screen reader announcement via a11y-push
+ *
+ * **Integration Points:**
+ *   - {@link module:core/js/views/notifyView} - Main notification view controller
+ *   - {@link module:core/js/views/notifyPopupView} - Modal popup renderer
+ *   - {@link module:core/js/views/notifyPushView} - Push notification renderer
+ *   - {@link module:core/js/a11y} - Accessibility integration
+ *
+ * **Public Events (plugins can listen to these):**
+ *   - `notify:opened` - Modal notification opened
+ *   - `notify:closed` - Modal notification closed
+ *   - `notify:cancelled` - Modal cancelled by user
+ *   - `notify:pushShown` - Push notification displayed
+ *   - `notify:pushRemoved` - Push notification removed
+ *
+ * **Dependencies:**
+ *   - {@link module:core/js/views/notifyView} - Core view implementation
+ *   - {@link module:core/js/a11y} - Screen reader integration
+ *   - Handlebars templates - `notifyPopup`, `notifyPush`
+ *
+ * **Extraction Viability:** Medium
+ *   - Could be extracted to `adapt-contrib-notify` plugin
+ *   - Requires: Config integration, a11y coordination, template system access
+ *   - Breaking Change Risk: High - Widely used throughout framework and plugins
+ *
+ * **Known Issues & Improvements:**
+ * - **Issue:** Modal stack edge case - Rapid open/close can leave orphaned modals (rare)
+ * - **Issue:** Push positioning - Assumes fixed navigation bar height (breaks with custom navs)
+ * - **Issue:** API inconsistency - `read()` returns promise, but `popup()`/`alert()` don't
+ * - **Enhancement:** Could return promises from all methods for better async control
+ * - **Enhancement:** Add `notify.closeAll()` method for bulk dismissal
+ * - **Enhancement:** Support notification queueing for modals (currently push only)
+ *
+ * @example
+ * import notify from 'core/js/notify';
+ *
+ * notify.popup({
+ *   title: 'Welcome',
+ *   body: 'Welcome to the course!'
+ * });
+ *
+ * notify.alert({
+ *   title: 'Confirm',
+ *   body: 'Are you sure?',
+ *   confirmText: 'Yes',
+ *   _callbackEvent: 'myPlugin:confirmed'
+ * });
+ *
+ * notify.push({
+ *   title: 'New message',
+ *   body: 'You have completed this page',
+ *   _timeout: 5000
+ * });
+ *
+ * await notify.read('Page navigation complete');
+ *
+ * // Customization examples
+ * notify.popup({
+ *   title: 'Warning',
+ *   body: 'Check your work',
+ *   _classes: 'my-custom-warning'  // Add CSS class for styling
+ * });
+ *
+ * notify.popup({
+ *   title: 'Custom Content',
+ *   _view: new MyCustomView({ model: myModel })  // Inject Backbone.View
+ * });
+ *
+ * // Listen to events for tracking
+ * Adapt.on('notify:closed', (notifyView) => {
+ *   console.log('User closed notification');
+ * });
+ *
+ * // Override defaults in config.json:
+ * // "_notify": { "_duration": 5000 }
+ */
+
 import NotifyView from 'core/js/views/notifyView';
 
+/**
+ * Global notification service singleton instance
+ * @type {NotifyView}
+ */
 const notify = new NotifyView();
+
 export default notify;