Skip to content

WEB-806: added persistent popups#3256

Merged
IOhacker merged 1 commit intoopenMF:devfrom
Nightwing-77:persistant-notifications
Mar 8, 2026
Merged

WEB-806: added persistent popups#3256
IOhacker merged 1 commit intoopenMF:devfrom
Nightwing-77:persistant-notifications

Conversation

@Nightwing-77
Copy link
Contributor

@Nightwing-77 Nightwing-77 commented Mar 2, 2026

Description

modified timer logic in notifications to have persistence if user hovers over it also modified the webapp component for this functionality

Related issues and discussion

WEB-806

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • [*] If you have multiple commits please combine them into one commit by squashing them.

  • [*] Read and understood the contribution guidelines at web-app/.github/CONTRIBUTING.md.

Summary by CodeRabbit

  • New Features
    • Notifications tray pauses automatic refresh while you hover and resumes when the cursor leaves, preventing interruptions while you read.
    • Alert/snackbar messages now pause their auto-dismiss timer on hover and resume on mouseleave, giving you more time to read and interact before they disappear.

@coderabbitai
Copy link

coderabbitai bot commented Mar 2, 2026

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key(s) in object: 'pre_merge_checks'
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Walkthrough

Adds hover-driven pause/resume for notification polling and snackbar auto-dismiss: hovering the notifications tray pauses the fetch timer and resumes (triggering a fetch) on leave; snackbars open with no auto-dismiss and pause/resume their dismissal timer on hover.

Changes

Cohort / File(s) Summary
Notifications Tray
src/app/shared/notifications-tray/notifications-tray.component.html, src/app/shared/notifications-tray/notifications-tray.component.ts
Wires mouseenter / mouseleave on the notifications container to pauseTimer() / resumeTimer(); adds timerPaused state, clears existing timeouts before fetching, and schedules polling only when not paused.
Snackbar Hover Control
src/app/web-app.component.ts
Adds MatSnackBarRef usage, opens snackbars with duration: 0 (no auto-dismiss), and implements handleSnackbarHover to pause/resume snackbar dismissal via a hover-aware timer and resume with the default duration.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant NotificationsTray
    participant NotificationsService
    participant Timer

    User->>NotificationsTray: mouseenter (hover)
    NotificationsTray->>Timer: pauseTimer() — clear timeout
    User->>NotificationsTray: mouseleave
    NotificationsTray->>Timer: resumeTimer() — set timeout
    Timer->>NotificationsService: fetchUnreadNotifications()
    NotificationsService-->>NotificationsTray: return unread notifications
Loading
sequenceDiagram
    participant User
    participant WebAppComponent
    participant MatSnackBar
    participant Timer

    WebAppComponent->>MatSnackBar: open(snackbar, duration=0) (store ref)
    MatSnackBar-->>User: show snackbar
    User->>MatSnackBar: mouseenter
    MatSnackBar->>Timer: pause auto-dismiss timer
    User->>MatSnackBar: mouseleave
    MatSnackBar->>Timer: start timer with defaultDuration
    Timer->>MatSnackBar: dismiss()
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • IOhacker
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title mentions 'persistent popups' but the changes actually implement hover-based timer pause/resume for notifications, not persistent popups. Update the title to accurately reflect the feature, such as 'Add hover-based pause/resume for notification and snackbar timers' or 'Implement pause-on-hover for notification timers'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
src/app/web-app.component.ts (1)

214-222: Bind alert stream subscription to component teardown

This subscription should be lifecycle-bound to avoid duplicate snackbar handlers when the component is recreated.

♻️ Suggested fix
-    this.alertService.alertEvent.subscribe((alertEvent: Alert) => {
+    this.alertService.alertEvent.pipe(takeUntil(this.destroy$)).subscribe((alertEvent: Alert) => {
       const snackBarRef = this.snackBar.open(`${alertEvent.message}`, 'Close', {
         duration: 0, // ✅ Set to 0 - no auto-dismiss initially
         horizontalPosition: 'right',
         verticalPosition: 'top'
       });
       // ✅ Handle hover behavior
       this.handleSnackbarHover(snackBarRef, 2000);
     });

As per coding guidelines: For Angular code in src/app/**, verify clean observable patterns.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/web-app.component.ts` around lines 214 - 222, The alertEvent
subscription in web-app.component.ts is not tied to component teardown and can
leak or duplicate handlers; modify the subscription made via
this.alertService.alertEvent.subscribe(...) to be lifecycle-bound by either
storing the Subscription returned (e.g., this.alertSub) and unsubscribing in
ngOnDestroy, or by piping the observable with takeUntil(this.destroy$) and
completing destroy$ in ngOnDestroy; ensure the logic that opens the snackBar and
calls this.handleSnackbarHover(snackBarRef, 2000) remains unchanged but is
executed only while the component is alive (update component class to declare
the Subscription or destroy$ and implement ngOnDestroy accordingly).
src/app/shared/notifications-tray/notifications-tray.component.html (1)

32-32: Add keyboard-equivalent pause/resume handlers

Pause/resume is mouse-only right now. Add focus handlers so keyboard navigation gets the same persistence behavior.

♿ Suggested fix
-  <div class="notifications-container" (mouseenter)="pauseTimer()" (mouseleave)="resumeTimer()">
+  <div
+    class="notifications-container"
+    (mouseenter)="pauseTimer()"
+    (mouseleave)="resumeTimer()"
+    (focusin)="pauseTimer()"
+    (focusout)="resumeTimer()">
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/shared/notifications-tray/notifications-tray.component.html` at line
32, The notifications container currently only binds mouse events; add
keyboard/focus equivalents so keyboard users get the same persistence by wiring
focus and blur to the existing pauseTimer() and resumeTimer() handlers on the
same element (the element with class "notifications-container" in
notifications-tray.component.html), and ensure the container is
keyboard-focusable (e.g., has a tabindex or appropriate interactive role) so
focus events will fire for keyboard navigation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/app/shared/notifications-tray/notifications-tray.component.ts`:
- Around line 128-131: resumeTimer currently flips timerPaused and calls
fetchUnreadNotifications unconditionally, which can spawn duplicate polling
loops; change resumeTimer to first check if timerPaused is false and return
early (i.e., only proceed when timerPaused is true), then set timerPaused =
false and call fetchUnreadNotifications; additionally modify
fetchUnreadNotifications to clear any existing timeout/interval handle before
scheduling a new one (store the handle on the component, e.g., this.pollTimeout
/ this.pollInterval) so multiple calls cannot stack, and ensure any existing
handle is cleared when pausing in pauseTimer.

In `@src/app/web-app.component.ts`:
- Around line 272-276: Replace the hardcoded setTimeout-based DOM probing with
the snackbar's lifecycle hook: where you currently call setTimeout(() => { const
snackbarContainer = document.querySelector('.mat-mdc-snack-bar-container'); ...
}, 100), subscribe to the MatSnackBarRef.afterOpened() observable (e.g.,
snackbarRef.afterOpened().subscribe(...)) and perform the
document.querySelector('.mat-mdc-snack-bar-container') lookup inside that
callback; if the lookup returns null, call snackbarRef.dismiss() instead of
silently returning. Ensure you reference the actual MatSnackBarRef instance used
in this component (snackbarRef or snackBarRef) and remove the setTimeout usage.

---

Nitpick comments:
In `@src/app/shared/notifications-tray/notifications-tray.component.html`:
- Line 32: The notifications container currently only binds mouse events; add
keyboard/focus equivalents so keyboard users get the same persistence by wiring
focus and blur to the existing pauseTimer() and resumeTimer() handlers on the
same element (the element with class "notifications-container" in
notifications-tray.component.html), and ensure the container is
keyboard-focusable (e.g., has a tabindex or appropriate interactive role) so
focus events will fire for keyboard navigation.

In `@src/app/web-app.component.ts`:
- Around line 214-222: The alertEvent subscription in web-app.component.ts is
not tied to component teardown and can leak or duplicate handlers; modify the
subscription made via this.alertService.alertEvent.subscribe(...) to be
lifecycle-bound by either storing the Subscription returned (e.g.,
this.alertSub) and unsubscribing in ngOnDestroy, or by piping the observable
with takeUntil(this.destroy$) and completing destroy$ in ngOnDestroy; ensure the
logic that opens the snackBar and calls this.handleSnackbarHover(snackBarRef,
2000) remains unchanged but is executed only while the component is alive
(update component class to declare the Subscription or destroy$ and implement
ngOnDestroy accordingly).

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 155aa7d and 65aa045.

📒 Files selected for processing (3)
  • src/app/shared/notifications-tray/notifications-tray.component.html
  • src/app/shared/notifications-tray/notifications-tray.component.ts
  • src/app/web-app.component.ts

Copy link
Contributor

@IOhacker IOhacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kindly see my comments

Copy link
Contributor

@IOhacker IOhacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kindly see my comments

@Nightwing-77 Nightwing-77 force-pushed the persistant-notifications branch from 65aa045 to 17939b8 Compare March 2, 2026 18:36
@Nightwing-77
Copy link
Contributor Author

Kindly see my comments

owwk!! fixed the comments!!

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/app/web-app.component.ts (2)

214-222: Add lifecycle teardown to alertEvent subscription.

Line 214 subscribes without teardown. Bind it to destroy$ for consistent observable cleanup.

Proposed fix
-    this.alertService.alertEvent.subscribe((alertEvent: Alert) => {
+    this.alertService.alertEvent.pipe(takeUntil(this.destroy$)).subscribe((alertEvent: Alert) => {
       const snackBarRef = this.snackBar.open(`${alertEvent.message}`, 'Close', {
         duration: 0, // Set to 0 - no auto-dismiss initially
         horizontalPosition: 'right',
         verticalPosition: 'top'
       });
       // Handle hover behavior
       this.handleSnackbarHover(snackBarRef, 2000);
     });

As per coding guidelines: "src/app/**: For Angular code: verify ... clean observable patterns."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/web-app.component.ts` around lines 214 - 222, The subscription to
alertService.alertEvent must be torn down using the component's destroy$
observable to avoid leaks; update the block where you subscribe
(alertService.alertEvent.subscribe(...)) to pipe the observable through
takeUntil(this.destroy$) (or equivalent) before subscribing, e.g.
alertService.alertEvent.pipe(takeUntil(this.destroy$)).subscribe(...) so the
stream is cancelled on ngOnDestroy; ensure destroy$ is defined as a Subject and
that ngOnDestroy calls destroy$.next() and destroy$.complete(); keep the
existing logic (snackBar.open and this.handleSnackbarHover(snackBarRef, 2000))
intact.

270-278: Tighten types in hover handler (any is avoidable).

Use concrete types for MatSnackBarRef and timer state instead of any.

Proposed fix
-import { MatSnackBar, MatSnackBarRef } from '@angular/material/snack-bar';
+import { MatSnackBar, MatSnackBarRef, TextOnlySnackBar } from '@angular/material/snack-bar';
...
-  private handleSnackbarHover(snackBarRef: MatSnackBarRef<any>, defaultDuration: number): void {
+  private handleSnackbarHover(snackBarRef: MatSnackBarRef<TextOnlySnackBar>, defaultDuration: number): void {
...
-      let dismissTimer: any;
+      let dismissTimer: ReturnType<typeof setTimeout> | null = null;

As per coding guidelines: "src/app/**: For Angular code: verify ... strict type safety ..."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/web-app.component.ts` around lines 270 - 278, The hover handler
currently uses broad any types; change the parameter type from
MatSnackBarRef<any> to MatSnackBarRef<unknown> (or a specific snack component
type if you have one) and tighten the timer variable by declaring let
dismissTimer: number | null = null (or use ReturnType<typeof setTimeout> if you
prefer) and update the clearTimeout/assignment checks to handle null safely
(e.g., if (dismissTimer !== null) clearTimeout(dismissTimer); dismissTimer =
window.setTimeout(...)). This updates handleSnackbarHover and its use of
dismissTimer to avoid any.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/app/web-app.component.ts`:
- Around line 214-222: The subscription to alertService.alertEvent must be torn
down using the component's destroy$ observable to avoid leaks; update the block
where you subscribe (alertService.alertEvent.subscribe(...)) to pipe the
observable through takeUntil(this.destroy$) (or equivalent) before subscribing,
e.g. alertService.alertEvent.pipe(takeUntil(this.destroy$)).subscribe(...) so
the stream is cancelled on ngOnDestroy; ensure destroy$ is defined as a Subject
and that ngOnDestroy calls destroy$.next() and destroy$.complete(); keep the
existing logic (snackBar.open and this.handleSnackbarHover(snackBarRef, 2000))
intact.
- Around line 270-278: The hover handler currently uses broad any types; change
the parameter type from MatSnackBarRef<any> to MatSnackBarRef<unknown> (or a
specific snack component type if you have one) and tighten the timer variable by
declaring let dismissTimer: number | null = null (or use ReturnType<typeof
setTimeout> if you prefer) and update the clearTimeout/assignment checks to
handle null safely (e.g., if (dismissTimer !== null) clearTimeout(dismissTimer);
dismissTimer = window.setTimeout(...)). This updates handleSnackbarHover and its
use of dismissTimer to avoid any.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 65aa045 and 17939b8.

📒 Files selected for processing (3)
  • src/app/shared/notifications-tray/notifications-tray.component.html
  • src/app/shared/notifications-tray/notifications-tray.component.ts
  • src/app/web-app.component.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/app/shared/notifications-tray/notifications-tray.component.ts

@Nightwing-77 Nightwing-77 requested a review from IOhacker March 2, 2026 18:41
@Nightwing-77 Nightwing-77 force-pushed the persistant-notifications branch from 17939b8 to 1fc8962 Compare March 3, 2026 04:40
@Nightwing-77 Nightwing-77 requested a review from IOhacker March 3, 2026 04:42
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/app/shared/notifications-tray/notifications-tray.component.ts (1)

138-141: ⚠️ Potential issue | 🟠 Major

Guard resume to avoid redundant immediate fetches when already running.

If resumeTimer() is triggered while not paused, Line 140 still forces another fetch cycle. Add an early return.

🐛 Proposed fix
 resumeTimer = () => {
+  if (!this.timerPaused) {
+    return;
+  }
   this.timerPaused = false;
   this.fetchUnreadNotifications();
 };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/shared/notifications-tray/notifications-tray.component.ts` around
lines 138 - 141, The resumeTimer function should no-op when the timer is already
running to avoid redundant immediate fetches; update resumeTimer in
notifications-tray.component.ts (the method named resumeTimer referencing
this.timerPaused and this.fetchUnreadNotifications) to check if this.timerPaused
is false and return early, otherwise set this.timerPaused = false and call
this.fetchUnreadNotifications().
🧹 Nitpick comments (1)
src/app/shared/notifications-tray/notifications-tray.component.ts (1)

133-136: Clear the handle after timeout cancellation for cleaner state.

After Line 135, set this.timer = null so follow-up checks don’t treat a stale handle as active.

♻️ Proposed cleanup
 pauseTimer = () => {
   this.timerPaused = true;
   clearTimeout(this.timer);
+  this.timer = null;
 };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/shared/notifications-tray/notifications-tray.component.ts` around
lines 133 - 136, The pauseTimer arrow method leaves a stale timeout handle in
this.timer after clearTimeout, so update pauseTimer (the pauseTimer = () => {
... } function) to also set this.timer = null after clearTimeout(this.timer) and
keep this.timerPaused = true; to ensure subsequent logic treating this.timer as
active won't be misled by an old handle; locate the pauseTimer method in
notifications-tray.component.ts and add the assignment to null immediately after
the clearTimeout call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/app/shared/notifications-tray/notifications-tray.component.ts`:
- Around line 113-116: The code is creating duplicate timers which can survive
pauseTimer(); before scheduling a new timeout ensure any existing timer is
cleared and only schedule when not paused — i.e., in the method that currently
does the unconditional setTimeout (look for the block that sets this.timer
around lines 122/126) first call the existing clear logic (the if (this.timer) {
clearTimeout(this.timer); this.timer = null; }) then guard the setTimeout with a
paused check (only assign this.timer = setTimeout(...) when !this.isPaused /
!this.paused or when pauseTimer() isn’t active), and remove the second
unconditional scheduling so there is a single code path that both clears
previous timers and sets a new one only when not paused.

---

Duplicate comments:
In `@src/app/shared/notifications-tray/notifications-tray.component.ts`:
- Around line 138-141: The resumeTimer function should no-op when the timer is
already running to avoid redundant immediate fetches; update resumeTimer in
notifications-tray.component.ts (the method named resumeTimer referencing
this.timerPaused and this.fetchUnreadNotifications) to check if this.timerPaused
is false and return early, otherwise set this.timerPaused = false and call
this.fetchUnreadNotifications().

---

Nitpick comments:
In `@src/app/shared/notifications-tray/notifications-tray.component.ts`:
- Around line 133-136: The pauseTimer arrow method leaves a stale timeout handle
in this.timer after clearTimeout, so update pauseTimer (the pauseTimer = () => {
... } function) to also set this.timer = null after clearTimeout(this.timer) and
keep this.timerPaused = true; to ensure subsequent logic treating this.timer as
active won't be misled by an old handle; locate the pauseTimer method in
notifications-tray.component.ts and add the assignment to null immediately after
the clearTimeout call.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 17939b8 and 1fc8962.

📒 Files selected for processing (3)
  • src/app/shared/notifications-tray/notifications-tray.component.html
  • src/app/shared/notifications-tray/notifications-tray.component.ts
  • src/app/web-app.component.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/app/shared/notifications-tray/notifications-tray.component.html
  • src/app/web-app.component.ts

@Nightwing-77 Nightwing-77 force-pushed the persistant-notifications branch from 1fc8962 to e6943ba Compare March 4, 2026 06:40
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
src/app/shared/notifications-tray/notifications-tray.component.ts (1)

137-140: ⚠️ Potential issue | 🟠 Major

Make resumeTimer idempotent to avoid redundant fetches.

Line 137 resumes unconditionally. If mouseleave fires while already resumed, this can still trigger unnecessary immediate refetches.

Suggested change
   resumeTimer = () => {
+    if (!this.timerPaused) {
+      return;
+    }
     this.timerPaused = false;
     this.fetchUnreadNotifications();
   };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/shared/notifications-tray/notifications-tray.component.ts` around
lines 137 - 140, The resumeTimer handler should be made idempotent to avoid
redundant immediate fetches: inside the existing resumeTimer arrow function
(resumeTimer) check the current flag (timerPaused) and only flip it to false and
call fetchUnreadNotifications() when timerPaused is actually true; if
timerPaused is already false, return early and do nothing. Keep the function as
an arrow property so binding is preserved and only perform the state change and
fetch when necessary.
🧹 Nitpick comments (2)
src/app/web-app.component.ts (2)

270-282: Tighten types in snackbar hover handling (any usage).

Line 270 and Line 281 use any in new logic. This weakens type safety in a changed path.

Suggested change
-import { MatSnackBar, MatSnackBarRef } from '@angular/material/snack-bar';
+import { MatSnackBar, MatSnackBarRef, TextOnlySnackBar } from '@angular/material/snack-bar';
@@
-  private handleSnackbarHover(snackBarRef: MatSnackBarRef<any>, defaultDuration: number): void {
+  private handleSnackbarHover(snackBarRef: MatSnackBarRef<TextOnlySnackBar>, defaultDuration: number): void {
@@
-        let dismissTimer: any;
+        let dismissTimer: ReturnType<typeof setTimeout> | null = null;

As per coding guidelines src/app/**: “verify ... strict type safety.”

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/web-app.component.ts` around lines 270 - 282, The local variable
dismissTimer in handleSnackbarHover is typed as any; tighten it to a proper
timer type such as ReturnType<typeof setTimeout> | undefined (or number |
undefined) and update any clearTimeout/assignment sites accordingly so
TypeScript knows it's a timer handle; also cast the result of
document.querySelector('.mat-mdc-snack-bar-container') to the appropriate DOM
type (e.g., HTMLElement | null) instead of using implicit any so
snackbarContainer has a strict type when checked and manipulated in
handleSnackbarHover.

214-222: Add teardown to the alert stream subscription.

Line 214 subscribes directly to alertEvent without lifecycle teardown. Align this with existing takeUntil(this.destroy$) usage in this component.

Suggested change
-    this.alertService.alertEvent.subscribe((alertEvent: Alert) => {
+    this.alertService.alertEvent.pipe(takeUntil(this.destroy$)).subscribe((alertEvent: Alert) => {
       const snackBarRef = this.snackBar.open(`${alertEvent.message}`, 'Close', {
         duration: 0, // Set to 0 - no auto-dismiss initially
         horizontalPosition: 'right',
         verticalPosition: 'top'
       });
       // Handle hover behavior
       this.handleSnackbarHover(snackBarRef, 2000);
     });

As per coding guidelines src/app/**: “verify ... clean observable patterns.”

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/web-app.component.ts` around lines 214 - 222, The subscription to
alertService.alertEvent should be torn down using the existing destroy$ pattern:
pipe the observable with takeUntil(this.destroy$) before calling subscribe
(i.e., alertService.alertEvent.pipe(takeUntil(this.destroy$)).subscribe(...)) so
the snackBar handling in handleSnackbarHover(snackBarRef, 2000) is cleaned up on
component destroy; also ensure destroy$ is completed in ngOnDestroy
(this.destroy$.next(); this.destroy$.complete()) if not already present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/app/shared/notifications-tray/notifications-tray.component.ts`:
- Around line 137-140: The resumeTimer handler should be made idempotent to
avoid redundant immediate fetches: inside the existing resumeTimer arrow
function (resumeTimer) check the current flag (timerPaused) and only flip it to
false and call fetchUnreadNotifications() when timerPaused is actually true; if
timerPaused is already false, return early and do nothing. Keep the function as
an arrow property so binding is preserved and only perform the state change and
fetch when necessary.

---

Nitpick comments:
In `@src/app/web-app.component.ts`:
- Around line 270-282: The local variable dismissTimer in handleSnackbarHover is
typed as any; tighten it to a proper timer type such as ReturnType<typeof
setTimeout> | undefined (or number | undefined) and update any
clearTimeout/assignment sites accordingly so TypeScript knows it's a timer
handle; also cast the result of
document.querySelector('.mat-mdc-snack-bar-container') to the appropriate DOM
type (e.g., HTMLElement | null) instead of using implicit any so
snackbarContainer has a strict type when checked and manipulated in
handleSnackbarHover.
- Around line 214-222: The subscription to alertService.alertEvent should be
torn down using the existing destroy$ pattern: pipe the observable with
takeUntil(this.destroy$) before calling subscribe (i.e.,
alertService.alertEvent.pipe(takeUntil(this.destroy$)).subscribe(...)) so the
snackBar handling in handleSnackbarHover(snackBarRef, 2000) is cleaned up on
component destroy; also ensure destroy$ is completed in ngOnDestroy
(this.destroy$.next(); this.destroy$.complete()) if not already present.

ℹ️ Review info
Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0e018801-d682-41d5-a020-dfcfb9eec583

📥 Commits

Reviewing files that changed from the base of the PR and between 1fc8962 and e6943ba.

📒 Files selected for processing (3)
  • src/app/shared/notifications-tray/notifications-tray.component.html
  • src/app/shared/notifications-tray/notifications-tray.component.ts
  • src/app/web-app.component.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/app/shared/notifications-tray/notifications-tray.component.html

Copy link
Contributor

@IOhacker IOhacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@IOhacker IOhacker merged commit 586ad2d into openMF:dev Mar 8, 2026
5 checks passed
@Nightwing-77 Nightwing-77 deleted the persistant-notifications branch March 9, 2026 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants