From 49d351f8dcd1b95e97b22cd249f395db93b4cb21 Mon Sep 17 00:00:00 2001 From: Joseph Replin Date: Mon, 5 Jan 2026 17:01:59 -0600 Subject: [PATCH 1/7] Add JSDoc comments to notification modules Added detailed JSDoc documentation to notification-related files, including models, collections, views, and the main notification service. The comments describe module responsibilities, public APIs, integration points, known issues, and usage examples, improving code maintainability and developer onboarding. --- js/collections/notifyPushCollection.js | 69 +++++- js/models/notifyModel.js | 63 ++++++ js/notify.js | 96 ++++++++ js/views/notifyPopupView.js | 214 ++++++++++++++++++ js/views/notifyPushView.js | 103 +++++++++ js/views/notifyView.js | 301 +++++++++++++++++++++---- 6 files changed, 806 insertions(+), 40 deletions(-) diff --git a/js/collections/notifyPushCollection.js b/js/collections/notifyPushCollection.js index 6881ddcf..75294df4 100644 --- a/js/collections/notifyPushCollection.js +++ b/js/collections/notifyPushCollection.js @@ -1,8 +1,57 @@ +/** + * @file Push Notification Collection - Manages queue of push notifications + * @module core/js/collections/notifyPushCollection + * @description Collection managing the queue and display of push notifications. + * Ensures maximum of 2 push notifications are visible simultaneously. + * + * **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 NotifyPushView for rendering + * - Listens to Adapt global event bus + * - Expects models to have `_isActive` and `_delay` properties + * + * **Known Issues & Improvements:** + * - ⚠️ **No queue limit**: Could queue unlimited notifications (memory leak risk) + * - ⚠️ **Race condition**: Rapid add/remove can trigger duplicate displays + * - ⚠️ **Hardcoded limit**: Max 2 visible is not configurable + * - 💡 **Improvement**: Add max queue size with overflow handling + * - 💡 **Improvement**: Make visible limit configurable via `_maxVisible` option + * - 💡 **Improvement**: Add `clearQueue()` method to dismiss all queued notifications + * - 💡 **Improvement**: Return view instance from `showPush()` for external control + * + * @example + * import NotifyPushCollection from 'core/js/collections/notifyPushCollection'; + * + * const pushes = new NotifyPushCollection(); + * + * pushes.add(new NotifyModel({ + * title: 'First', + * body: 'First notification', + * _timeout: 3000, + * _delay: 500 // Optional: wait 500ms before showing + * })); + */ + 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} + */ export default class NotifyPushCollection extends Backbone.Collection { initialize() { @@ -21,11 +70,29 @@ export default class NotifyPushCollection extends Backbone.Collection { this.showPush(model); } + /** + * Determines if another push notification can be shown. + * @returns {boolean} True if fewer than 2 active notifications + * @private + */ canShowPush() { const availablePushNotifications = this.where({ _isActive: true }); return (availablePushNotifications.length < 2); } + /** + * Displays a push notification by creating its view. + * **Side effect:** Creates new NotifyPushView instance. + * @param {NotifyModel} model - The notification model to display + * @param {number} [model._delay=0] - Milliseconds to wait before showing + * @private + * @example + * const model = new NotifyModel({ + * body: 'Delayed message', + * _delay: 1000 + * }); + * collection.add(model); + */ showPush(model) { _.delay(() => { new NotifyPushView({ diff --git a/js/models/notifyModel.js b/js/models/notifyModel.js index 2006c39d..a8cbf7c1 100644 --- a/js/models/notifyModel.js +++ b/js/models/notifyModel.js @@ -1,5 +1,46 @@ +/** + * @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:** + * - ⚠️ **No validation**: Accepts any property without schema validation + * - ⚠️ **Silent failures**: Invalid `_timeout` or `_delay` values don't throw errors + * - 💡 **Improvement**: Add property validation for `_timeout`/`_delay` (must be numbers >= 0) + * - 💡 **Improvement**: Add `isActive()` getter method instead of direct property access + * - 💡 **Improvement**: Support cancellation tokens for `onClosed()` promise + * + * @example + * import NotifyModel from 'core/js/models/notifyModel'; + * + * const notification = new NotifyModel({ + * title: 'Alert', + * body: 'Important message', + * _timeout: 5000 + * }); + * + * await notification.onClosed(); + */ + 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 +53,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} 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)); diff --git a/js/notify.js b/js/notify.js index f31909df..a0c76c46 100644 --- a/js/notify.js +++ b/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:** + * - ⚠️ **Modal stack edge case**: Rapid open/close can leave orphaned modals (rare) + * - ⚠️ **Push positioning**: Assumes fixed navigation bar height (breaks with custom navs) + * - 💡 **Improvement**: Could return promises from all methods for better async control + * - 💡 **Improvement**: Add `notify.closeAll()` method for bulk dismissal + * - 💡 **Improvement**: Support notification queueing for modals (currently push only) + * - 💡 **API inconsistency**: `read()` returns promise, but `popup()`/`alert()` don't + * + * @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; diff --git a/js/views/notifyPopupView.js b/js/views/notifyPopupView.js index ca6daf5b..63754377 100644 --- a/js/views/notifyPopupView.js +++ b/js/views/notifyPopupView.js @@ -1,3 +1,60 @@ +/** + * @file Popup Modal View - Renders popup, alert, and prompt notifications + * @module core/js/views/notifyPopupView + * @description View responsible for rendering and managing modal popup notifications + * displayed in the center of the screen with shadow overlay. Handles three notification + * types: popup (custom content), alert (single button), and prompt (multiple buttons). + * + * **Modal Types:** + * - **popup**: Custom content view (ComponentView or custom view) + * - **alert**: Simple notification with OK button and callback + * - **prompt**: Question with multiple action buttons + * + * **Features:** + * - Stack-based modal management (multiple modals supported) + * - Focus management and keyboard navigation + * - Shadow click to close (configurable) + * - Escape key to cancel (configurable) + * - Automatic resizing and responsive layout + * - Scroll locking when modal open + * - Animation support (can be disabled) + * - Subview rendering with automatic component lookup + * + * **Lifecycle:** + * 1. Created by NotifyView.create() + * 2. Rendered with shadow overlay + * 3. Subview rendered (if specified) + * 4. Focus moved to first accessible element + * 5. User interaction or programmatic close + * 6. Focus restored to previous element + * 7. View removed from DOM + * + * **Known Issues & Improvements:** + * - ⚠️ **Focus restoration**: Can fail if original element was removed from DOM + * - ⚠️ **Animation timing**: Disabled animation still has CSS transition delay + * - ⚠️ **Resize handling**: Excessive resize calculations on every window resize + * - ⚠️ **Memory leak risk**: Event listeners not always cleaned up on rapid close + * - 💡 **Improvement**: Debounce resize handler to reduce calculations + * - 💡 **Improvement**: Use IntersectionObserver for visibility detection + * - 💡 **Improvement**: Add `_maxHeight` option to constrain popup size + * - 💡 **Improvement**: Support `_position` option (top/center/bottom) + * + * @example + * // Popup with custom view + * import NotifyPopupView from 'core/js/views/notifyPopupView'; + * import NotifyModel from 'core/js/models/notifyModel'; + * + * const model = new NotifyModel({ + * _view: myCustomView, + * _closeOnShadowClick: true + * }); + * + * const popupView = new NotifyPopupView({ + * model, + * notify: notifyViewInstance + * }); + */ + import Adapt from 'core/js/adapt'; import components from 'core/js/components'; import data from 'core/js/data'; @@ -6,6 +63,17 @@ import AdaptView from 'core/js/views/adaptView'; import Backbone from 'backbone'; import { transitionNextFrame, transitionsEnded } from '../transitions'; +/** + * @class NotifyPopupView + * @classdesc Stack-based modals (only top closeable). Focus managed (moved on open, restored on close). + * @extends {Backbone.View} + * @example + * // Constructor (normally called by NotifyView internally) + * const popup = new NotifyPopupView({ + * model: notifyModel, + * notify: notifyViewInstance // Required: parent NotifyView for stack management + * }); + */ export default class NotifyPopupView extends Backbone.View { className() { @@ -27,10 +95,14 @@ export default class NotifyPopupView extends Backbone.View { initialize({ notify }) { this.notify = notify; _.bindAll(this, 'onShadowClicked', 'resetNotifySize', 'onKeyDown'); + this.disableAnimation = Adapt.config.get('_disableAnimation') ?? false; this.$el.toggleClass('disable-animation', Boolean(this.disableAnimation)); + this.isOpen = false; + this.hasOpened = false; + this.setupEventListeners(); this.render(); const dialog = this.$('.notify__popup')[0]; @@ -51,6 +123,12 @@ export default class NotifyPopupView extends Backbone.View { $(window).on('keydown', this.onKeyDown); } + /** + * Handles keydown events for Escape key cancellation. + * Only triggers if focus is within the popup. + * @param {jQuery.Event} event - Keyboard event + * @private + */ onKeyDown(event) { if (event.which !== 27) return; const isFocusInPopup = Boolean($(document.activeElement).closest(this.$el).length); @@ -67,6 +145,23 @@ export default class NotifyPopupView extends Backbone.View { return this; } + /** + * Handles alert button click. + * Closes popup and triggers callback event specified in model. + * @param {jQuery.Event} event - Click event + * @example + * // In notification creation: + * notify.alert({ + * title: 'Confirmation', + * body: 'Your action was successful', + * _callbackEvent: 'action:confirmed' + * }); + * + * // Handler: + * Adapt.on('action:confirmed', (notifyView) => { + * console.log('User clicked OK'); + * }); + */ onAlertButtonClicked(event) { event.preventDefault(); // tab index preservation, notify must close before subsequent callback is triggered @@ -74,6 +169,29 @@ export default class NotifyPopupView extends Backbone.View { Adapt.trigger(this.model.get('_callbackEvent'), this); } + /** + * Handles prompt button click. + * Closes popup and triggers event specified in button's `data-event` attribute. + * @param {jQuery.Event} event - Click event + * @example + * // In notification creation: + * notify.prompt({ + * title: 'Confirm Action', + * body: 'Are you sure?', + * _prompts: [ + * { promptText: 'Yes', _callbackEvent: 'action:confirmed' }, + * { promptText: 'No', _callbackEvent: 'action:cancelled' } + * ] + * }); + * + * // Handlers: + * Adapt.on('action:confirmed', (notifyView) => { + * console.log('User clicked Yes'); + * }); + * Adapt.on('action:cancelled', (notifyView) => { + * console.log('User clicked No'); + * }); + */ onPromptButtonClicked(event) { event.preventDefault(); // tab index preservation, notify must close before subsequent callback is triggered @@ -87,6 +205,18 @@ export default class NotifyPopupView extends Backbone.View { this.cancelNotify(); } + /** + * Handles clicks on the shadow overlay outside the popup. + * Closes popup if `_closeOnShadowClick` is true (default). + * @param {MouseEvent} event - Native mouse event + * @private + * @example + * // Prevent shadow click from closing: + * notify.popup({ + * _view: myView, + * _closeOnShadowClick: false + * }); + */ onShadowClicked(event) { const dialog = this.$('.notify__popup')[0]; const rect = dialog.getBoundingClientRect(); @@ -98,6 +228,19 @@ export default class NotifyPopupView extends Backbone.View { this.cancelNotify(); } + /** + * Cancels the notification without triggering callback. + * Respects `_isCancellable` flag - cannot cancel if false. + * @fires notify:cancelled + * @example + * Adapt.trigger('notify:cancel'); + * + * notify.alert({ + * title: 'Required', + * body: 'You must read this', + * _isCancellable: false + * }); + */ cancelNotify() { if (this.model.get('_isCancellable') === false) return; // tab index preservation, notify must close before subsequent callback is triggered @@ -110,6 +253,11 @@ export default class NotifyPopupView extends Backbone.View { this.resizeNotify(); } + /** + * Recalculates and applies popup dimensions. + * Makes popup full-screen with scroll if content exceeds viewport height. + * @private + */ resizeNotify() { const windowHeight = $(window).height(); const notifyHeight = this.$('.notify__popup-inner').outerHeight(); @@ -121,6 +269,13 @@ export default class NotifyPopupView extends Backbone.View { }); } + /** + * Displays the notification with animation. + * Renders subview, adds to stack, and waits for images to load. + * @async + * @fires notify:opened + * @private + */ async showNotify() { this.isOpen = true; await this.addSubView(); @@ -130,6 +285,12 @@ export default class NotifyPopupView extends Backbone.View { this.$el.imageready(this.onLoaded.bind(this)); } + /** + * Handles post-load setup after images have loaded. + * Sets up focus, scroll locking, and animation. + * @async + * @private + */ async onLoaded() { this.hasOpened = true; // Allows popup manager to control focus @@ -147,6 +308,32 @@ export default class NotifyPopupView extends Backbone.View { await transitionsEnded(this.$('.notify__popup, .notify__shadow')); } + /** + * Renders subview if specified in model. + * Supports three patterns: + * 1. Custom view passed via `_view` property + * 2. Component auto-render via `_shouldRenderId` and `_id` + * 3. No subview (text-only notification) + * @async + * @private + * @example + * // Pattern 1: Custom view + * notify.popup({ + * _view: new MyView({ model: myModel }) + * }); + * + * // Pattern 2: Auto-render component + * notify.popup({ + * _id: 'c-05', + * _shouldRenderId: true + * }); + * + * // Pattern 3: Text only + * notify.alert({ + * title: 'Hello', + * body: 'World' + * }); + */ async addSubView() { this.subView = this.model.get('_view'); if (this.model.get('_shouldRenderId') && this.model.get('_id')) { @@ -170,6 +357,17 @@ export default class NotifyPopupView extends Backbone.View { }); } + /** + * Closes the popup notification. + * Only closes if this is the top-most popup in the stack. + * Waits for opening animation to complete if necessary. + * @example + * // Programmatically close notification: + * Adapt.trigger('notify:close'); + * + * // Or close specific notification: + * notifyPopupView.closeNotify(); + */ closeNotify() { // Make sure that only the top most notify is closed const stackItem = this.notify.stack[this.notify.stack.length - 1]; @@ -190,6 +388,12 @@ export default class NotifyPopupView extends Backbone.View { }); } + /** + * Handles close animation and cleanup. + * @async + * @fires notify:closed + * @private + */ async onCloseReady() { this.$el.addClass('anim-hide-before'); await transitionNextFrame(); @@ -206,6 +410,11 @@ export default class NotifyPopupView extends Backbone.View { Adapt.trigger('notify:closed', this); } + /** + * Cleanup when view is removed. + * Removes event listeners and cleans up subview. + * @param {...*} args - Arguments passed to Backbone's remove method + */ remove(...args) { this.removeSubView(); this.el.removeEventListener('mousedown', this.onShadowClicked, { capture: true }); @@ -213,6 +422,11 @@ export default class NotifyPopupView extends Backbone.View { super.remove(...args); } + /** + * Removes subview and cleans up nested views. + * For AdaptView subviews, recursively removes all descendants. + * @private + */ removeSubView() { if (!this.subView) return; this.subView.$el.off('resize', this.resetNotifySize); diff --git a/js/views/notifyPushView.js b/js/views/notifyPushView.js index b5236a9d..8677889d 100644 --- a/js/views/notifyPushView.js +++ b/js/views/notifyPushView.js @@ -1,9 +1,57 @@ +/** + * @file Push Notification View - Renders individual push notifications + * @module core/js/views/notifyPushView + * @description View responsible for rendering and managing individual push notifications + * in the top-right corner of the viewport. Handles auto-close timing, positioning, + * click callbacks, and accessibility. + * + * **Display Behavior:** + * - Appears in top-right corner + * - Maximum 2 visible simultaneously (managed by {@link NotifyPushCollection}) + * - Auto-closes after timeout + * - Can be manually closed via close button + * - Uses `` element for semantic HTML + * - Animated entrance/exit via CSS transitions + * + * **Positioning:** + * - Top notification: below navigation bar + offset + * - Second notification: below first notification + offset + * - Automatically repositions when notifications added/removed + * + * **Known Issues & Improvements:** + * - ⚠️ **Hardcoded positioning**: Assumes navigation bar exists and is fixed height + * - ⚠️ **No mobile optimization**: Top-right position may be obscured on mobile + * - ⚠️ **Race condition**: updatePushPosition() can be called before render completes + * - ⚠️ **No Z-index management**: Multiple notifications can overlap with other UI + * - 💡 **Improvement**: Make position configurable (_position: 'top-right' | 'top-left' | 'bottom-right') + * - 💡 **Improvement**: Calculate nav height dynamically instead of hardcoding + * - 💡 **Improvement**: Add mobile-specific positioning (bottom of screen) + * - 💡 **Improvement**: Support `_priority` to control stacking order + * + * @example + * import NotifyPushView from 'core/js/views/notifyPushView'; + * import NotifyModel from 'core/js/models/notifyModel'; + * + * const model = new NotifyModel({ + * title: 'Success', + * body: 'Your changes have been saved', + * _timeout: 5000 + * }); + * + * const pushView = new NotifyPushView({ model }); + */ + import Adapt from 'core/js/adapt'; import a11y from '../a11y'; import { transitionsEnded } from '../transitions'; +/** + * @class NotifyPushView + * @classdesc Lifecycle: Created → Delayed render → Positioned → Auto-closed → Removed + * @extends {Backbone.View} + */ export default class NotifyPushView extends Backbone.View { tagName() { @@ -41,10 +89,21 @@ export default class NotifyPushView extends Backbone.View { this.render(); } + /** + * Sets up Escape key listener for keyboard accessibility. + * Allows users to dismiss push notification with Esc key. + * @private + */ setupEscapeKey() { $(window).on('keydown', this.onKeyDown); } + /** + * Handles keydown events for Escape key dismissal. + * Only triggers if focus is within the push notification. + * @param {jQuery.Event} event - Keyboard event + * @private + */ onKeyDown(event) { if (event.which !== 27) return; const isFocusInPopup = Boolean($(document.activeElement).closest(this.$el).length); @@ -84,6 +143,17 @@ export default class NotifyPushView extends Backbone.View { Adapt.trigger('notify:pushShown'); } + /** + * Closes and removes the push notification. + * Handles animation, accessibility cleanup, and model removal. + * Can be triggered by: timeout, close button, Esc key, or notification click. + * @async + * @param {jQuery.Event} [event] - Click event if triggered by user interaction + * @example + * pushView.closePush(); + * + * _.delay(() => this.closePush(), this.model.get('_timeout')); + */ async closePush(event) { if (event) { event.preventDefault(); @@ -103,11 +173,33 @@ export default class NotifyPushView extends Backbone.View { } } + /** + * Triggers the callback event associated with this notification. + * Called when user clicks the notification body (not the close button). + * @param {jQuery.Event} event - Click event + * @example + * // In notification creation: + * notify.push({ + * title: 'New Message', + * body: 'Click to view', + * _callbackEvent: 'messages:show' + * }); + * + * // Handler: + * Adapt.on('messages:show', () => { + * console.log('User clicked notification'); + * }); + */ triggerEvent(event) { Adapt.trigger(this.model.get('_callbackEvent')); this.closePush(); } + /** + * Updates position index for all active push notifications. + * Recalculates and applies index to each active notification. + * @private + */ updateIndexPosition() { if (this.hasBeenRemoved) return; const models = this.model.collection.models; @@ -118,6 +210,12 @@ export default class NotifyPushView extends Backbone.View { }); } + /** + * Updates the vertical position of this push notification. + * Positions based on index (0 = top, 1 = below first notification). + * Takes navigation bar height into account. + * @private + */ updatePushPosition() { if (this.hasBeenRemoved) { return; @@ -139,6 +237,11 @@ export default class NotifyPushView extends Backbone.View { } } + /** + * Cleanup when view is removed. + * Removes global event listeners. + * @param {...*} args - Arguments passed to Backbone's remove method + */ remove(...args) { $(window).off('keydown', this.onKeyDown); super.remove(...args); diff --git a/js/views/notifyView.js b/js/views/notifyView.js index f7ba15d7..a5dc5303 100644 --- a/js/views/notifyView.js +++ b/js/views/notifyView.js @@ -1,3 +1,63 @@ +/** + * @file Notification View Controller - Main notification system controller + * @module core/js/views/notifyView + * @description Central controller for the notification system. Manages all notification types + * (popup, alert, prompt, push) and their display lifecycle. Coordinates between models, + * views, and collections to provide a unified notification API. + * + * **Notification Types:** + * - **popup**: Modal overlay with custom content + * - **alert**: Modal with title, body, and confirm button + * - **prompt**: Modal with title, body, and multiple action buttons + * - **push**: Non-blocking notification in top-right corner (max 2 visible) + * - **a11y-push**: Screen reader announcement (invisible) + * + * **Integration Points:** + * - {@link module:core/js/models/notifyModel} - Notification data + * - {@link module:core/js/views/notifyPopupView} - Modal rendering + * - {@link module:core/js/views/notifyPushView} - Push rendering + * - {@link module:core/js/collections/notifyPushCollection} - Push queue management + * - {@link module:core/js/a11y} - Accessibility coordination + * + * **Known Issues & Improvements:** + * - ⚠️ **Stack management**: Rapid open/close can orphan modals in stack + * - ⚠️ **Deprecated API**: Still supports event-based API (`notify:popup`) for backwards compatibility + * - ⚠️ **No promise support**: `popup()`, `alert()`, `prompt()` don't return promises (only `read()` does) + * - 💡 **Improvement**: Unify API - all methods should return promises for consistency + * - 💡 **Improvement**: Add `closeAll()` method for bulk dismissal + * - 💡 **Improvement**: Add modal queueing like push notifications (currently unlimited stack) + * - 💡 **Improvement**: Remove deprecated event-based API in v7.0 + * - 💡 **Performance**: Stack array manipulation could be optimized with Set + * + * @example + * import notify from 'core/js/notify'; + * + * // Popup with custom styling + * notify.popup({ + * title: 'Welcome', + * body: 'Course introduction', + * _classes: 'welcome-popup' + * }); + * + * // Alert with confirmation callback + * notify.alert({ + * title: 'Confirm Delete', + * body: 'This cannot be undone.', + * confirmText: 'Delete', + * _callbackEvent: 'item:deleted' + * }); + * + * // Push notification (auto-closes after timeout) + * notify.push({ + * title: 'Page Complete', + * body: 'Great job!', + * _timeout: 5000 + * }); + * + * // Announce to screen readers + * await notify.read('Loading complete'); + */ + import Adapt from 'core/js/adapt'; import logging from 'core/js/logging'; import NotifyPushCollection from 'core/js/collections/notifyPushCollection'; @@ -5,6 +65,11 @@ import NotifyPopupView from 'core/js/views/notifyPopupView'; import NotifyModel from 'core/js/models/notifyModel'; import a11y from '../a11y'; +/** + * @class NotifyView + * @classdesc Modal notifications stack (only topmost closeable). Push notifications queue (max 2 visible). + * @extends {Backbone.View} + */ export default class NotifyView extends Backbone.View { className() { @@ -13,7 +78,9 @@ export default class NotifyView extends Backbone.View { initialize() { this._stack = []; + this.notifyPushes = new NotifyPushCollection(); + this.listenTo(Adapt, { 'app:dataReady': this.onDataReady, 'notify:popup': this._deprecated.bind(this, 'popup'), @@ -29,14 +96,38 @@ export default class NotifyView extends Backbone.View { document.documentElement.style.setProperty('--adapt-notify-duration', `${notifyDuration}ms`); } + /** + * Gets the current modal notification stack. + * @returns {NotifyPopupView[]} Array of open modal notifications + * @example + * const openModals = notify.stack; + * console.log(`${openModals.length} modals open`); + */ get stack() { return this._stack; } + /** + * Checks if any modal notifications are currently open. + * @returns {boolean} True if one or more modals are open + * @example + * if (notify.isOpen) { + * console.log('Modal is open'); + * } + */ get isOpen() { return (this.stack.length > 0); } + /** + * Handles deprecated event-based notification API. + * Logs deprecation warning and delegates to new API. + * @param {string} type - Notification type (popup/alert/prompt/push) + * @param {Object} notifyObject - Notification options + * @returns {NotifyModel|NotifyPopupView} Created notification instance + * @deprecated Use `notify.popup()`, `notify.alert()`, etc. directly + * @private + */ _deprecated(type, notifyObject) { logging.deprecated(`NOTIFY DEPRECATED: Adapt.trigger('notify:${type}', notifyObject); is no longer supported, please use notify.${type}(notifyObject);`); return this.create(notifyObject, { _type: type }); @@ -48,6 +139,18 @@ export default class NotifyView extends Backbone.View { this.$el.appendTo('body'); } + /** + * Creates a notification instance of the specified type. + * Central factory method for all notification types. + * @param {Object} notifyObject - Notification configuration options + * @param {string} notifyObject.title - Notification title + * @param {string} notifyObject.body - Notification body text/HTML + * @param {string} [notifyObject._classes] - Additional CSS classes + * @param {Object} defaults - Default options to merge with notifyObject + * @param {string} defaults._type - Notification type (popup/alert/prompt/push/a11y-push) + * @returns {NotifyModel|NotifyPopupView} Created notification instance + * @private + */ create(notifyObject, defaults) { notifyObject = _.defaults({}, notifyObject, defaults, { _type: 'popup', @@ -75,69 +178,189 @@ export default class NotifyView extends Backbone.View { } /** - * Creates a 'popup' notify - * @param {Object} notifyObject An object containing all the settings for the popup - * @param {string} notifyObject.title Title of the popup - * @param {string} notifyObject.body Body of the popup - * @param {Boolean} [notifyObject._showCloseButton=true] If set to `false` the popup will not have a close button. The learner will still be able to dismiss the popup by clicking outside of it or by pressing the Esc key. This setting is typically used mainly for popups that have a subview (where the subview contains the close button) - * @param {Boolean} [notifyObject._isCancellable=true] If set to `false` the learner will not be able to close the popup - use with caution! - * @param {string} [notifyObject._classes] A class name or (space separated) list of class names you'd like to be applied to the popup's `
` - * @param {Backbone.View} [notifyObject._view] Subview to display in the popup instead of the standard view + * Creates a 'popup' notify - a modal overlay with custom content. + * @param {Object} notifyObject - Configuration object for the popup + * @param {string} notifyObject.title - Title of the popup + * @param {string} notifyObject.body - Body content of the popup (HTML supported) + * @param {boolean} [notifyObject._showCloseButton=true] - If set to `false` the popup will not have a close button. The learner will still be able to dismiss the popup by clicking outside of it or by pressing the Esc key. This setting is typically used mainly for popups that have a subview (where the subview contains the close button) + * @param {boolean} [notifyObject._isCancellable=true] - If set to `false` the learner will not be able to close the popup - use with caution! + * @param {boolean} [notifyObject._closeOnShadowClick=true] - Whether clicking outside popup closes it + * @param {string} [notifyObject._classes] - A class name or (space separated) list of class names you'd like to be applied to the popup's `
` + * @param {Backbone.View} [notifyObject._view] - Subview to display in the popup instead of the standard view + * @param {Object} [notifyObject._attributes] - HTML attributes to apply to popup element + * @param {string} [notifyObject._id] - Content ID to auto-render (when `_shouldRenderId` is true) + * @param {boolean} [notifyObject._shouldRenderId=false] - Auto-render content model by ID + * @returns {NotifyPopupView} The created popup view instance + * @example + * import notify from 'core/js/notify'; + * + * notify.popup({ + * title: 'Information', + * body: '

This is important information.

' + * }); + * + * notify.popup({ + * title: 'Warning', + * body: 'Please review before proceeding.', + * _classes: 'warning-popup' + * }); + * + * notify.popup({ + * title: 'Custom Content', + * _view: new MyCustomView() + * }); */ popup(notifyObject) { return this.create(notifyObject, { _type: 'popup' }); } /** - * Creates an 'alert' notify popup - * @param {Object} notifyObject An object containing all the settings for the alert popup - * @param {string} notifyObject.title Title of the alert popup - * @param {string} notifyObject.body Body of the alert popup - * @param {string} notifyObject.confirmText Label for the popup confirm button - * @param {Boolean} [notifyObject._isCancellable=true] If set to `false` only the confirm button can be used to dismiss/close the popup - * @param {Boolean} [notifyObject._showIcon=false] If set to `true` an 'alert' icon will be displayed in the popup - * @param {string} [notifyObject._callbackEvent] Event to trigger when the confirm button is clicked - * @param {string} [notifyObject._classes] A class name or (space separated) list of class names you'd like to be applied to the popup's `
` - * @param {Backbone.View} [notifyObject._view] Subview to display in the popup instead of the standard view + * Creates an 'alert' notify popup - a modal with a confirm button and optional callback. + * @param {Object} notifyObject - Configuration object for the alert popup + * @param {string} notifyObject.title - Title of the alert popup + * @param {string} notifyObject.body - Body content of the alert popup + * @param {string} notifyObject.confirmText - Label for the popup confirm button + * @param {boolean} [notifyObject._isCancellable=true] - If set to `false` only the confirm button can be used to dismiss/close the popup + * @param {boolean} [notifyObject._showIcon=false] - If set to `true` an 'alert' icon will be displayed in the popup + * @param {string} [notifyObject._callbackEvent] - Event to trigger when the confirm button is clicked + * @param {string} [notifyObject._classes] - A class name or (space separated) list of class names you'd like to be applied to the popup's `
` + * @param {Backbone.View} [notifyObject._view] - Subview to display in the popup instead of the standard view + * @returns {NotifyPopupView} The created alert view instance + * @example + * import notify from 'core/js/notify'; + * + * // Simple alert + * notify.alert({ + * title: 'Complete', + * body: 'You have finished this section.', + * confirmText: 'Continue' + * }); + * + * // Alert with callback + * notify.alert({ + * title: 'Confirm Action', + * body: 'Are you sure you want to reset your progress?', + * confirmText: 'Yes, Reset', + * _callbackEvent: 'course:reset', + * _showIcon: true + * }); + * + * Adapt.on('course:reset', () => { + * console.log('User confirmed reset'); + * }); */ alert(notifyObject) { return this.create(notifyObject, { _type: 'alert' }); } /** - * Creates a 'prompt dialog' notify popup - * @param {Object} notifyObject An object containing all the settings for the prompt dialog - * @param {string} notifyObject.title Title of the prompt - * @param {string} notifyObject.body Body of the prompt - * @param {Object[]} notifyObject._prompts Array of objects that each define a button (and associated callback event) that you want shown in the prompt - * @param {string} notifyObject._prompts[].promptText Label for this button - * @param {string} notifyObject._prompts[]._callbackEvent Event to be triggered when this button is clicked - * @param {Boolean} [notifyObject._isCancellable=true] If set to `false` only the confirm button can be used to dismiss/close the prompt - * @param {Boolean} [notifyObject._showIcon=true] If set to `true` a 'query' icon will be displayed in the popup - * @param {string} [notifyObject._callbackEvent] Event to trigger when the confirm button is clicked - * @param {string} [notifyObject._classes] A class name or (space separated) list of class names you'd like to be applied to the popup's `
` - * @param {Backbone.View} [notifyObject._view] Subview to display in the popup instead of the standard view + * Creates a 'prompt dialog' notify popup - a modal with multiple custom action buttons. + * @param {Object} notifyObject - Configuration object for the prompt dialog + * @param {string} notifyObject.title - Title of the prompt + * @param {string} notifyObject.body - Body content of the prompt + * @param {Object[]} notifyObject._prompts - Array of objects that each define a button (and associated callback event) that you want shown in the prompt + * @param {string} notifyObject._prompts[].promptText - Label for this button + * @param {string} notifyObject._prompts[]._callbackEvent - Event to be triggered when this button is clicked + * @param {boolean} [notifyObject._isCancellable=true] - If set to `false` only the action buttons can be used to dismiss/close the prompt + * @param {boolean} [notifyObject._showIcon=true] - If set to `true` a 'query' icon will be displayed in the popup + * @param {string} [notifyObject._callbackEvent] - Event to trigger when the confirm button is clicked (deprecated - use _prompts) + * @param {string} [notifyObject._classes] - A class name or (space separated) list of class names you'd like to be applied to the popup's `
` + * @param {Backbone.View} [notifyObject._view] - Subview to display in the popup instead of the standard view + * @returns {NotifyPopupView} The created prompt view instance + * @example + * import notify from 'core/js/notify'; + * + * notify.prompt({ + * title: 'Choose an Option', + * body: 'How would you like to proceed?', + * _prompts: [ + * { + * promptText: 'Save and Continue', + * _callbackEvent: 'course:save' + * }, + * { + * promptText: 'Continue Without Saving', + * _callbackEvent: 'course:continue' + * }, + * { + * promptText: 'Exit Course', + * _callbackEvent: 'course:exit' + * } + * ], + * _isCancellable: false, + * _showIcon: true + * }); + * + * Adapt.on('course:save', () => { + * console.log('User chose to save'); + * }); */ prompt(notifyObject) { return this.create(notifyObject, { _type: 'prompt' }); } /** - * Creates a 'push notification' - * @param {Object} notifyObject An object containing all the settings for the push notification - * @param {string} notifyObject.title Title of the push notification - * @param {string} notifyObject.body Body of the push notification - * @param {Number} [notifyObject._timeout=3000] Length of time (in milliseconds) the notification should left on-screen before automatically fading away - * @param {string} notifyObject._callbackEvent Event to be triggered if the learner clicks on the push notification (not triggered if they use the close button) - * @param {string} [notifyObject._classes] A class name or (space separated) list of class names you'd like to be applied to the popup's `
` + * Creates a 'push notification' - a non-blocking notification in the top-right corner. + * Push notifications auto-close after a timeout and up to 2 can be shown simultaneously. + * @param {Object} notifyObject - Configuration object for the push notification + * @param {string} notifyObject.title - Title of the push notification + * @param {string} notifyObject.body - Body content of the push notification + * @param {number} [notifyObject._timeout=3000] - Length of time (in milliseconds) the notification should be displayed before automatically fading away + * @param {number} [notifyObject._delay=0] - Delay (in milliseconds) before showing the notification + * @param {string} [notifyObject._callbackEvent] - Event to be triggered if the learner clicks on the push notification (not triggered if they use the close button) + * @param {string} [notifyObject._classes] - A class name or (space separated) list of class names you'd like to be applied to the notification's `
` + * @returns {NotifyModel} The created notification model (queued for display) + * @example + * import notify from 'core/js/notify'; + * + * // Simple push notification + * notify.push({ + * title: 'Page Complete', + * body: 'You have completed this page.', + * _timeout: 5000 + * }); + * + * // Push with click callback + * notify.push({ + * title: 'New Achievement', + * body: 'Click to view your badges.', + * _callbackEvent: 'badges:show', + * _timeout: 8000 + * }); + * + * Adapt.on('badges:show', () => { + * console.log('User clicked push notification'); + * }); + * + * // Delayed push + * notify.push({ + * title: 'Reminder', + * body: 'Don't forget to complete the assessment.', + * _delay: 10000, // Show after 10 seconds + * _timeout: 5000 + * }); */ push(notifyObject) { return this.create(notifyObject, { _type: 'push' }); } /** - * Creates and waits for an a11y-push notify to read. - * @param {string} body + * Creates and waits for an a11y-push notification to be read by screen readers. + * This creates an invisible notification that announces text to assistive technology. + * Automatically calculates display duration based on text length (200ms per word + 150ms base + delay buffer). + * @async + * @param {string} body - Text to announce to screen readers + * @returns {Promise} Resolves when the announcement has had time to be read + * @example + * import notify from 'core/js/notify'; + * + * // Announce to screen readers + * await notify.read('Page navigation complete'); + * + * // Sequential announcements + * await notify.read('Loading content'); + * await asyncOperation(); + * await notify.read('Content loaded successfully'); */ async read (body) { // Allow time enough for the message to be read before focusing. From c6759838f00b9992af8e2ad4503a4d90873029ff Mon Sep 17 00:00:00 2001 From: Joseph Replin Date: Tue, 6 Jan 2026 14:32:34 -0600 Subject: [PATCH 2/7] Update NotifyPushCollection JSDoc --- js/collections/notifyPushCollection.js | 64 ++++++++++++++------------ 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/js/collections/notifyPushCollection.js b/js/collections/notifyPushCollection.js index 75294df4..ca50064f 100644 --- a/js/collections/notifyPushCollection.js +++ b/js/collections/notifyPushCollection.js @@ -1,8 +1,12 @@ /** * @file Push Notification Collection - Manages queue of push notifications * @module core/js/collections/notifyPushCollection - * @description Collection managing the queue and display of push notifications. - * Ensures maximum of 2 push notifications are visible simultaneously. + * + * 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) @@ -18,30 +22,23 @@ * - Delayed display via model `_delay` property * * **Dependencies:** - * - Requires NotifyPushView for rendering - * - Listens to Adapt global event bus + * - 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:** - * - ⚠️ **No queue limit**: Could queue unlimited notifications (memory leak risk) - * - ⚠️ **Race condition**: Rapid add/remove can trigger duplicate displays - * - ⚠️ **Hardcoded limit**: Max 2 visible is not configurable - * - 💡 **Improvement**: Add max queue size with overflow handling - * - 💡 **Improvement**: Make visible limit configurable via `_maxVisible` option - * - 💡 **Improvement**: Add `clearQueue()` method to dismiss all queued notifications - * - 💡 **Improvement**: Return view instance from `showPush()` for external control + * - **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 * - * @example - * import NotifyPushCollection from 'core/js/collections/notifyPushCollection'; + * **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)`. * - * const pushes = new NotifyPushCollection(); - * - * pushes.add(new NotifyModel({ - * title: 'First', - * body: 'First notification', - * _timeout: 3000, - * _delay: 500 // Optional: wait 500ms before showing - * })); + * @see {@link NotifyPushView} for rendering implementation */ import Adapt from 'core/js/adapt'; @@ -51,6 +48,11 @@ import NotifyModel from 'core/js/models/notifyModel'; /** * @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 { @@ -72,7 +74,11 @@ export default class NotifyPushCollection extends Backbone.Collection { /** * Determines if another push notification can be shown. - * @returns {boolean} True if fewer than 2 active notifications + * + * **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() { @@ -82,16 +88,16 @@ export default class NotifyPushCollection extends Backbone.Collection { /** * Displays a push notification by creating its view. - * **Side effect:** Creates new NotifyPushView instance. + * + * **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 - * @example - * const model = new NotifyModel({ - * body: 'Delayed message', - * _delay: 1000 - * }); - * collection.add(model); */ showPush(model) { _.delay(() => { From 58f424bb09bcfc391679583669d615050f955d68 Mon Sep 17 00:00:00 2001 From: Joseph Replin Date: Tue, 6 Jan 2026 14:36:45 -0600 Subject: [PATCH 3/7] Update notifyModel.js documentation --- js/models/notifyModel.js | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/js/models/notifyModel.js b/js/models/notifyModel.js index a8cbf7c1..f89d11b2 100644 --- a/js/models/notifyModel.js +++ b/js/models/notifyModel.js @@ -16,22 +16,15 @@ * - `_hasClosed` - Tracks if notification already closed * * **Known Issues & Improvements:** - * - ⚠️ **No validation**: Accepts any property without schema validation - * - ⚠️ **Silent failures**: Invalid `_timeout` or `_delay` values don't throw errors - * - 💡 **Improvement**: Add property validation for `_timeout`/`_delay` (must be numbers >= 0) - * - 💡 **Improvement**: Add `isActive()` getter method instead of direct property access - * - 💡 **Improvement**: Support cancellation tokens for `onClosed()` promise + * - **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 * - * @example - * import NotifyModel from 'core/js/models/notifyModel'; - * - * const notification = new NotifyModel({ - * title: 'Alert', - * body: 'Important message', - * _timeout: 5000 - * }); - * - * await notification.onClosed(); + * **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'; From 4fcd8daefc356da75022d0d231c63e55bf9b6e8a Mon Sep 17 00:00:00 2001 From: Joseph Replin Date: Tue, 6 Jan 2026 15:19:51 -0600 Subject: [PATCH 4/7] Update NotifyPopupView JSDoc --- js/views/notifyPopupView.js | 113 +++++++++++++++++------------------- 1 file changed, 52 insertions(+), 61 deletions(-) diff --git a/js/views/notifyPopupView.js b/js/views/notifyPopupView.js index 63754377..99ce7461 100644 --- a/js/views/notifyPopupView.js +++ b/js/views/notifyPopupView.js @@ -30,29 +30,18 @@ * 7. View removed from DOM * * **Known Issues & Improvements:** - * - ⚠️ **Focus restoration**: Can fail if original element was removed from DOM - * - ⚠️ **Animation timing**: Disabled animation still has CSS transition delay - * - ⚠️ **Resize handling**: Excessive resize calculations on every window resize - * - ⚠️ **Memory leak risk**: Event listeners not always cleaned up on rapid close - * - 💡 **Improvement**: Debounce resize handler to reduce calculations - * - 💡 **Improvement**: Use IntersectionObserver for visibility detection - * - 💡 **Improvement**: Add `_maxHeight` option to constrain popup size - * - 💡 **Improvement**: Support `_position` option (top/center/bottom) + * - **Issue:** Focus restoration - Can fail if original element was removed from DOM + * - **Issue:** Animation timing - Disabled animation still has CSS transition delay + * - **Issue:** Resize handling - Excessive resize calculations on every window resize + * - **Issue:** Memory leak risk - Event listeners not always cleaned up on rapid close + * - **Enhancement:** Debounce resize handler to reduce calculations + * - **Enhancement:** Use IntersectionObserver for visibility detection + * - **Enhancement:** Add `_maxHeight` option to constrain popup size + * - **Enhancement:** Support `_position` option (top/center/bottom) * - * @example - * // Popup with custom view - * import NotifyPopupView from 'core/js/views/notifyPopupView'; - * import NotifyModel from 'core/js/models/notifyModel'; - * - * const model = new NotifyModel({ - * _view: myCustomView, - * _closeOnShadowClick: true - * }); - * - * const popupView = new NotifyPopupView({ - * model, - * notify: notifyViewInstance - * }); + * **Important:** Do NOT manually instantiate with `new NotifyPopupView()`. + * Views are created internally by {@link NotifyView}. Use `notify.popup()`, `notify.alert()`, + * or `notify.prompt()` instead. */ import Adapt from 'core/js/adapt'; @@ -66,13 +55,8 @@ import { transitionNextFrame, transitionsEnded } from '../transitions'; /** * @class NotifyPopupView * @classdesc Stack-based modals (only top closeable). Focus managed (moved on open, restored on close). + * Created internally by NotifyView - do not instantiate directly. * @extends {Backbone.View} - * @example - * // Constructor (normally called by NotifyView internally) - * const popup = new NotifyPopupView({ - * model: notifyModel, - * notify: notifyViewInstance // Required: parent NotifyView for stack management - * }); */ export default class NotifyPopupView extends Backbone.View { @@ -150,17 +134,18 @@ export default class NotifyPopupView extends Backbone.View { * Closes popup and triggers callback event specified in model. * @param {jQuery.Event} event - Click event * @example - * // In notification creation: - * notify.alert({ + * const view = notify.alert({ * title: 'Confirmation', * body: 'Your action was successful', * _callbackEvent: 'action:confirmed' * }); * - * // Handler: - * Adapt.on('action:confirmed', (notifyView) => { + * const onConfirmed = (notifyView) => { + * if (notifyView !== view) return; * console.log('User clicked OK'); - * }); + * Adapt.off('action:confirmed', onConfirmed); + * }; + * Adapt.on('action:confirmed', onConfirmed); */ onAlertButtonClicked(event) { event.preventDefault(); @@ -174,8 +159,7 @@ export default class NotifyPopupView extends Backbone.View { * Closes popup and triggers event specified in button's `data-event` attribute. * @param {jQuery.Event} event - Click event * @example - * // In notification creation: - * notify.prompt({ + * const view = notify.prompt({ * title: 'Confirm Action', * body: 'Are you sure?', * _prompts: [ @@ -184,13 +168,22 @@ export default class NotifyPopupView extends Backbone.View { * ] * }); * - * // Handlers: - * Adapt.on('action:confirmed', (notifyView) => { + * const onConfirmed = (notifyView) => { + * if (notifyView !== view) return; * console.log('User clicked Yes'); - * }); - * Adapt.on('action:cancelled', (notifyView) => { + * cleanUp(); + * }; + * const onCancelled = (notifyView) => { + * if (notifyView !== view) return; * console.log('User clicked No'); - * }); + * cleanUp(); + * }; + * Adapt.on('action:confirmed', onConfirmed); + * Adapt.on('action:cancelled', onCancelled); + * function cleanUp() { + * Adapt.off('action:confirmed', onConfirmed); + * Adapt.off('action:cancelled', onCancelled); + * } */ onPromptButtonClicked(event) { event.preventDefault(); @@ -208,14 +201,9 @@ export default class NotifyPopupView extends Backbone.View { /** * Handles clicks on the shadow overlay outside the popup. * Closes popup if `_closeOnShadowClick` is true (default). + * Set `_closeOnShadowClick: false` to prevent closing on shadow click. * @param {MouseEvent} event - Native mouse event * @private - * @example - * // Prevent shadow click from closing: - * notify.popup({ - * _view: myView, - * _closeOnShadowClick: false - * }); */ onShadowClicked(event) { const dialog = this.$('.notify__popup')[0]; @@ -231,10 +219,12 @@ export default class NotifyPopupView extends Backbone.View { /** * Cancels the notification without triggering callback. * Respects `_isCancellable` flag - cannot cancel if false. + * + * Triggered by Escape key press or by `Adapt.trigger('notify:cancel')` event. + * Set `_isCancellable: false` to prevent cancellation. + * * @fires notify:cancelled * @example - * Adapt.trigger('notify:cancel'); - * * notify.alert({ * title: 'Required', * body: 'You must read this', @@ -310,25 +300,24 @@ export default class NotifyPopupView extends Backbone.View { /** * Renders subview if specified in model. - * Supports three patterns: - * 1. Custom view passed via `_view` property - * 2. Component auto-render via `_shouldRenderId` and `_id` - * 3. No subview (text-only notification) + * + * **Supports three patterns:** + * 1. **Custom view** - Pass Backbone.View via `_view` property + * 2. **Auto-render component** - Set `_shouldRenderId: true` and provide `_id` + * 3. **Text-only notification** - Omit `_view` and `_shouldRenderId` + * * @async * @private * @example - * // Pattern 1: Custom view * notify.popup({ * _view: new MyView({ model: myModel }) * }); * - * // Pattern 2: Auto-render component * notify.popup({ * _id: 'c-05', * _shouldRenderId: true * }); * - * // Pattern 3: Text only * notify.alert({ * title: 'Hello', * body: 'World' @@ -361,12 +350,14 @@ export default class NotifyPopupView extends Backbone.View { * Closes the popup notification. * Only closes if this is the top-most popup in the stack. * Waits for opening animation to complete if necessary. - * @example - * // Programmatically close notification: - * Adapt.trigger('notify:close'); - * - * // Or close specific notification: - * notifyPopupView.closeNotify(); + * + * **Note:** This is an internal method. Notifications close automatically when: + * - User clicks alert/prompt button + * - User clicks close button + * - User presses Escape key (if `_isCancellable` is true) + * - User clicks shadow overlay (if `_closeOnShadowClick` is true) + * + * @private */ closeNotify() { // Make sure that only the top most notify is closed From d53dd7265334b87d140ea9fe655ee657e712af54 Mon Sep 17 00:00:00 2001 From: Joseph Replin Date: Tue, 6 Jan 2026 15:27:40 -0600 Subject: [PATCH 5/7] Update NotifyPushView docs --- js/views/notifyPushView.js | 46 ++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/js/views/notifyPushView.js b/js/views/notifyPushView.js index 8677889d..58c5d480 100644 --- a/js/views/notifyPushView.js +++ b/js/views/notifyPushView.js @@ -19,26 +19,17 @@ * - Automatically repositions when notifications added/removed * * **Known Issues & Improvements:** - * - ⚠️ **Hardcoded positioning**: Assumes navigation bar exists and is fixed height - * - ⚠️ **No mobile optimization**: Top-right position may be obscured on mobile - * - ⚠️ **Race condition**: updatePushPosition() can be called before render completes - * - ⚠️ **No Z-index management**: Multiple notifications can overlap with other UI - * - 💡 **Improvement**: Make position configurable (_position: 'top-right' | 'top-left' | 'bottom-right') - * - 💡 **Improvement**: Calculate nav height dynamically instead of hardcoding - * - 💡 **Improvement**: Add mobile-specific positioning (bottom of screen) - * - 💡 **Improvement**: Support `_priority` to control stacking order + * - **Issue:** Hardcoded positioning - Assumes navigation bar exists and is fixed height + * - **Issue:** No mobile optimization - Top-right position may be obscured on mobile + * - **Issue:** Race condition - updatePushPosition() can be called before render completes + * - **Issue:** No Z-index management - Multiple notifications can overlap with other UI + * - **Enhancement:** Make position configurable (_position: 'top-right' | 'top-left' | 'bottom-right') + * - **Enhancement:** Calculate nav height dynamically instead of hardcoding + * - **Enhancement:** Add mobile-specific positioning (bottom of screen) + * - **Enhancement:** Support `_priority` to control stacking order * - * @example - * import NotifyPushView from 'core/js/views/notifyPushView'; - * import NotifyModel from 'core/js/models/notifyModel'; - * - * const model = new NotifyModel({ - * title: 'Success', - * body: 'Your changes have been saved', - * _timeout: 5000 - * }); - * - * const pushView = new NotifyPushView({ model }); + * **Important:** Do NOT manually instantiate with `new NotifyPushView()`. + * Views are created internally by {@link NotifyPushCollection}. Use `notify.push()` instead. */ import Adapt from 'core/js/adapt'; @@ -149,10 +140,7 @@ export default class NotifyPushView extends Backbone.View { * Can be triggered by: timeout, close button, Esc key, or notification click. * @async * @param {jQuery.Event} [event] - Click event if triggered by user interaction - * @example - * pushView.closePush(); - * - * _.delay(() => this.closePush(), this.model.get('_timeout')); + * @private */ async closePush(event) { if (event) { @@ -176,18 +164,22 @@ export default class NotifyPushView extends Backbone.View { /** * Triggers the callback event associated with this notification. * Called when user clicks the notification body (not the close button). + * Specify callback event using `_callbackEvent` option when creating notification. + * + * **Important:** Unlike `notify.popup()` which passes the view instance to callbacks, + * push notification event handlers receive NO arguments. The callback cannot identify + * which notification triggered it. Store model reference if identification needed. + * * @param {jQuery.Event} event - Click event * @example - * // In notification creation: - * notify.push({ + * const pushModel = notify.push({ * title: 'New Message', * body: 'Click to view', * _callbackEvent: 'messages:show' * }); * - * // Handler: * Adapt.on('messages:show', () => { - * console.log('User clicked notification'); + * console.log('Push notification clicked'); * }); */ triggerEvent(event) { From ce6ed47ed96ad5dd311bf13b318f68df1a64b648 Mon Sep 17 00:00:00 2001 From: Joseph Replin Date: Tue, 6 Jan 2026 15:28:16 -0600 Subject: [PATCH 6/7] Updated known issues and improvements sections --- js/notify.js | 12 ++++++------ js/views/notifyView.js | 44 ++++++++---------------------------------- 2 files changed, 14 insertions(+), 42 deletions(-) diff --git a/js/notify.js b/js/notify.js index a0c76c46..f88d7f27 100644 --- a/js/notify.js +++ b/js/notify.js @@ -38,12 +38,12 @@ * - Breaking Change Risk: High - Widely used throughout framework and plugins * * **Known Issues & Improvements:** - * - ⚠️ **Modal stack edge case**: Rapid open/close can leave orphaned modals (rare) - * - ⚠️ **Push positioning**: Assumes fixed navigation bar height (breaks with custom navs) - * - 💡 **Improvement**: Could return promises from all methods for better async control - * - 💡 **Improvement**: Add `notify.closeAll()` method for bulk dismissal - * - 💡 **Improvement**: Support notification queueing for modals (currently push only) - * - 💡 **API inconsistency**: `read()` returns promise, but `popup()`/`alert()` don't + * - **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'; diff --git a/js/views/notifyView.js b/js/views/notifyView.js index a5dc5303..d9b969fd 100644 --- a/js/views/notifyView.js +++ b/js/views/notifyView.js @@ -20,42 +20,14 @@ * - {@link module:core/js/a11y} - Accessibility coordination * * **Known Issues & Improvements:** - * - ⚠️ **Stack management**: Rapid open/close can orphan modals in stack - * - ⚠️ **Deprecated API**: Still supports event-based API (`notify:popup`) for backwards compatibility - * - ⚠️ **No promise support**: `popup()`, `alert()`, `prompt()` don't return promises (only `read()` does) - * - 💡 **Improvement**: Unify API - all methods should return promises for consistency - * - 💡 **Improvement**: Add `closeAll()` method for bulk dismissal - * - 💡 **Improvement**: Add modal queueing like push notifications (currently unlimited stack) - * - 💡 **Improvement**: Remove deprecated event-based API in v7.0 - * - 💡 **Performance**: Stack array manipulation could be optimized with Set - * - * @example - * import notify from 'core/js/notify'; - * - * // Popup with custom styling - * notify.popup({ - * title: 'Welcome', - * body: 'Course introduction', - * _classes: 'welcome-popup' - * }); - * - * // Alert with confirmation callback - * notify.alert({ - * title: 'Confirm Delete', - * body: 'This cannot be undone.', - * confirmText: 'Delete', - * _callbackEvent: 'item:deleted' - * }); - * - * // Push notification (auto-closes after timeout) - * notify.push({ - * title: 'Page Complete', - * body: 'Great job!', - * _timeout: 5000 - * }); - * - * // Announce to screen readers - * await notify.read('Loading complete'); + * - **Issue:** Stack management - Rapid open/close can orphan modals in stack + * - **Issue:** Deprecated API - Still supports event-based API (`notify:popup`) for backwards compatibility + * - **Issue:** No promise support - `popup()`, `alert()`, `prompt()` don't return promises (only `read()` does) + * - **Enhancement:** Unify API - all methods should return promises for consistency + * - **Enhancement:** Add `closeAll()` method for bulk dismissal + * - **Enhancement:** Add modal queueing like push notifications (currently unlimited stack) + * - **Enhancement:** Remove deprecated event-based API in v7.0 + * - **Enhancement:** Stack array manipulation could be optimized with Set */ import Adapt from 'core/js/adapt'; From c21c29bacaaa50f01e5ba87a0e3791402c6d9872 Mon Sep 17 00:00:00 2001 From: Joseph Replin Date: Tue, 6 Jan 2026 15:36:22 -0600 Subject: [PATCH 7/7] Update notifyPopupView.js --- js/views/notifyPopupView.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/js/views/notifyPopupView.js b/js/views/notifyPopupView.js index 99ce7461..2fb52fcc 100644 --- a/js/views/notifyPopupView.js +++ b/js/views/notifyPopupView.js @@ -219,10 +219,10 @@ export default class NotifyPopupView extends Backbone.View { /** * Cancels the notification without triggering callback. * Respects `_isCancellable` flag - cannot cancel if false. - * + * * Triggered by Escape key press or by `Adapt.trigger('notify:cancel')` event. * Set `_isCancellable: false` to prevent cancellation. - * + * * @fires notify:cancelled * @example * notify.alert({ @@ -300,12 +300,12 @@ export default class NotifyPopupView extends Backbone.View { /** * Renders subview if specified in model. - * + * * **Supports three patterns:** * 1. **Custom view** - Pass Backbone.View via `_view` property * 2. **Auto-render component** - Set `_shouldRenderId: true` and provide `_id` * 3. **Text-only notification** - Omit `_view` and `_shouldRenderId` - * + * * @async * @private * @example @@ -350,13 +350,13 @@ export default class NotifyPopupView extends Backbone.View { * Closes the popup notification. * Only closes if this is the top-most popup in the stack. * Waits for opening animation to complete if necessary. - * + * * **Note:** This is an internal method. Notifications close automatically when: * - User clicks alert/prompt button * - User clicks close button * - User presses Escape key (if `_isCancellable` is true) * - User clicks shadow overlay (if `_closeOnShadowClick` is true) - * + * * @private */ closeNotify() {