Conversation
📝 WalkthroughWalkthroughAdded notification dismiss functionality across the desktop application. Translation keys for "dismiss" action added in English, Spanish, and Japanese locales. WidgetToast component now includes a dismiss button. useWidgetNotifications refactored to track active toasts and synchronize pass-through mouse interaction state via a custom event broadcast. Changes
Sequence DiagramsequenceDiagram
participant User
participant WidgetToast as WidgetToast<br/>(UI)
participant useWidgetNotifications as useWidgetNotifications<br/>(Hook)
participant Toast as Toast Library
participant FloatingButton as FloatingButton<br/>(Component)
User->>WidgetToast: Click dismiss button
WidgetToast->>useWidgetNotifications: Call onDismiss()
useWidgetNotifications->>Toast: Remove toast by ID
useWidgetNotifications->>useWidgetNotifications: Remove toastId from<br/>active set
useWidgetNotifications->>useWidgetNotifications: Call sync() to update<br/>pass-through state
useWidgetNotifications->>FloatingButton: Dispatch TOAST_INTERACTION_STATE_EVENT<br/>(active=false)
FloatingButton->>FloatingButton: Update hasActiveToastRef<br/>and enable mouse passthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/desktop/src/renderer/widget/pages/widget/components/FloatingButton.tsx (1)
12-12: Consider extracting duplicated event name constant.
TOAST_INTERACTION_STATE_EVENTis defined both here and inuseWidgetNotifications.tsx. Extract to a shared constants file to ensure consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/widget/pages/widget/components/FloatingButton.tsx` at line 12, TOAST_INTERACTION_STATE_EVENT is duplicated between FloatingButton.tsx and useWidgetNotifications.tsx; extract this string into a shared constant (e.g., export const TOAST_INTERACTION_STATE_EVENT) in a new or existing shared constants module and import that constant into both FloatingButton.tsx and useWidgetNotifications.tsx, replacing the local definitions so both files reference the single source of truth.apps/desktop/src/renderer/widget/hooks/useWidgetNotifications.tsx (1)
101-106: Missing dependency in cleanup effect.The cleanup effect calls
setIgnoreMouseEvents.mutatebut has an empty dependency array. While this works becausemutateis stable in tRPC, adding the linter disable comment or includingsetIgnoreMouseEventsin dependencies would clarify intent.♻️ Suggested improvement
useEffect(() => { return () => { activeToastIdsRef.current.clear(); setIgnoreMouseEvents.mutate({ ignore: true }); }; + // eslint-disable-next-line react-hooks/exhaustive-deps -- cleanup only, mutate is stable }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/widget/hooks/useWidgetNotifications.tsx` around lines 101 - 106, The cleanup effect in useWidgetNotifications uses setIgnoreMouseEvents.mutate and clears activeToastIdsRef.current but has an empty dependency array; update the effect to either include setIgnoreMouseEvents in the dependency array or add an explicit ESLint disable comment with a short justification (e.g., "mutate is stable") to make the intent explicit. Locate the useEffect cleanup that references activeToastIdsRef and setIgnoreMouseEvents and adjust the dependencies accordingly so the linter is satisfied and the code intent is clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/constants/models.ts`:
- Line 149: The referenced asset paths (e.g., providerIcon:
"/assets/icon_logo.svg" and other model entries using "/icons/models/local.svg")
are missing; update each model definition in models.ts that uses providerIcon
and related icon properties to point to existing asset paths or add the missing
SVG files to the repo. Locate the objects that define providerIcon (and any
other icon fields in the models array) and either correct the string values to
valid asset locations used by the app or add the corresponding SVG files at the
specified paths so imports resolve at runtime; ensure all occurrences of
"/assets/icon_logo.svg" and "/icons/models/local.svg" in models.ts are handled
consistently.
---
Nitpick comments:
In `@apps/desktop/src/renderer/widget/hooks/useWidgetNotifications.tsx`:
- Around line 101-106: The cleanup effect in useWidgetNotifications uses
setIgnoreMouseEvents.mutate and clears activeToastIdsRef.current but has an
empty dependency array; update the effect to either include setIgnoreMouseEvents
in the dependency array or add an explicit ESLint disable comment with a short
justification (e.g., "mutate is stable") to make the intent explicit. Locate the
useEffect cleanup that references activeToastIdsRef and setIgnoreMouseEvents and
adjust the dependencies accordingly so the linter is satisfied and the code
intent is clear.
In `@apps/desktop/src/renderer/widget/pages/widget/components/FloatingButton.tsx`:
- Line 12: TOAST_INTERACTION_STATE_EVENT is duplicated between
FloatingButton.tsx and useWidgetNotifications.tsx; extract this string into a
shared constant (e.g., export const TOAST_INTERACTION_STATE_EVENT) in a new or
existing shared constants module and import that constant into both
FloatingButton.tsx and useWidgetNotifications.tsx, replacing the local
definitions so both files reference the single source of truth.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
apps/desktop/public/assets/icon_logo.svgis excluded by!**/*.svgapps/desktop/public/assets/icon_logo@3x.pngis excluded by!**/*.pngapps/desktop/public/icons/models/local.svgis excluded by!**/*.svg
📒 Files selected for processing (8)
apps/desktop/src/constants/models.tsapps/desktop/src/i18n/locales/en.jsonapps/desktop/src/i18n/locales/es.jsonapps/desktop/src/i18n/locales/ja.jsonapps/desktop/src/renderer/widget/components/ToasterWrapper.tsxapps/desktop/src/renderer/widget/components/WidgetToast.tsxapps/desktop/src/renderer/widget/hooks/useWidgetNotifications.tsxapps/desktop/src/renderer/widget/pages/widget/components/FloatingButton.tsx
apps/desktop/src/constants/models.ts
Outdated
| setup: "cloud", | ||
| provider: "Amical Cloud", | ||
| providerIcon: "/assets/logo.svg", | ||
| providerIcon: "/assets/icon_logo.svg", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the referenced icon assets
echo "Searching for local.svg:"
find . -name "local.svg" -type f 2>/dev/null
echo -e "\nSearching for icon_logo.svg:"
find . -name "icon_logo.svg" -type f 2>/dev/null
echo -e "\nSearching for any SVG files in assets directory:"
find ./assets -name "*.svg" -type f 2>/dev/null | head -20
echo -e "\nSearching for any SVG files in icons directory:"
find ./icons -name "*.svg" -type f 2>/dev/null | head -20Repository: amicalhq/amical
Length of output: 212
Critical issue: Referenced asset files do not exist in the repository.
The code changes reference two assets that are missing:
/icons/models/local.svg— not found/assets/icon_logo.svg— not found
These files must be added to the repository or the paths must be corrected to reference existing assets. The UI will fail to load these icons at runtime without the asset files in place.
Also applies to: 180-181, 212-213, 245-246, 277-278, 309-310, 341-342
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/constants/models.ts` at line 149, The referenced asset paths
(e.g., providerIcon: "/assets/icon_logo.svg" and other model entries using
"/icons/models/local.svg") are missing; update each model definition in
models.ts that uses providerIcon and related icon properties to point to
existing asset paths or add the missing SVG files to the repo. Locate the
objects that define providerIcon (and any other icon fields in the models array)
and either correct the string values to valid asset locations used by the app or
add the corresponding SVG files at the specified paths so imports resolve at
runtime; ensure all occurrences of "/assets/icon_logo.svg" and
"/icons/models/local.svg" in models.ts are handled consistently.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/renderer/widget/hooks/useWidgetNotifications.tsx`:
- Around line 101-106: Cleanup currently clears activeToastIdsRef and sets
backend pass-through via setIgnoreMouseEvents.mutate({ ignore: true }) but does
not notify listeners that toast interaction ended; update the useEffect cleanup
to also emit a final inactive toast-state event (e.g., call the same
event/mutation used elsewhere to notify listeners of active state — search for
and invoke the existing notifier such as emitToastActiveState(false) or
setToastInteractionActive.mutate({ active: false })) so local listeners are
updated to active=false before unmount; keep the
activeToastIdsRef.current.clear() and setIgnoreMouseEvents.mutate call and add
the notification call after them.
In `@apps/desktop/src/renderer/widget/pages/widget/components/FloatingButton.tsx`:
- Around line 81-87: The unmount cleanup in the FloatingButton component
currently removes the TOAST_INTERACTION_STATE_EVENT listener
(handleToastInteractionState) but doesn't clear a pending debounce timeout,
allowing leaveTimeoutRef to fire after unmount; update the cleanup returned by
the useEffect to check leaveTimeoutRef.current and call
clearTimeout(leaveTimeoutRef.current) and then set leaveTimeoutRef.current =
null (or undefined) so any pending timeout is cancelled and cannot run stale
state/mutation logic after unmount.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/desktop/src/i18n/locales/en.jsonapps/desktop/src/i18n/locales/es.jsonapps/desktop/src/i18n/locales/ja.jsonapps/desktop/src/renderer/widget/components/ToasterWrapper.tsxapps/desktop/src/renderer/widget/components/WidgetToast.tsxapps/desktop/src/renderer/widget/hooks/useWidgetNotifications.tsxapps/desktop/src/renderer/widget/pages/widget/components/FloatingButton.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/i18n/locales/en.json
- apps/desktop/src/renderer/widget/components/ToasterWrapper.tsx
Comment on lines
+101
to
+106
| useEffect(() => { | ||
| return () => { | ||
| activeToastIdsRef.current.clear(); | ||
| setIgnoreMouseEvents.mutate({ ignore: true }); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
Emit a final inactive toast-state event on cleanup.
Cleanup resets backend pass-through, but it does not notify other listeners that toast interaction is now inactive. That can leave local listeners with stale active=true state after unmount.
💡 Suggested fix
useEffect(() => {
return () => {
activeToastIdsRef.current.clear();
+ window.dispatchEvent(
+ new CustomEvent<{ active: boolean }>(TOAST_INTERACTION_STATE_EVENT, {
+ detail: { active: false },
+ }),
+ );
setIgnoreMouseEvents.mutate({ ignore: true });
};
}, []);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| return () => { | |
| activeToastIdsRef.current.clear(); | |
| setIgnoreMouseEvents.mutate({ ignore: true }); | |
| }; | |
| }, []); | |
| useEffect(() => { | |
| return () => { | |
| activeToastIdsRef.current.clear(); | |
| window.dispatchEvent( | |
| new CustomEvent<{ active: boolean }>(TOAST_INTERACTION_STATE_EVENT, { | |
| detail: { active: false }, | |
| }), | |
| ); | |
| setIgnoreMouseEvents.mutate({ ignore: true }); | |
| }; | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/widget/hooks/useWidgetNotifications.tsx` around
lines 101 - 106, Cleanup currently clears activeToastIdsRef and sets backend
pass-through via setIgnoreMouseEvents.mutate({ ignore: true }) but does not
notify listeners that toast interaction ended; update the useEffect cleanup to
also emit a final inactive toast-state event (e.g., call the same event/mutation
used elsewhere to notify listeners of active state — search for and invoke the
existing notifier such as emitToastActiveState(false) or
setToastInteractionActive.mutate({ active: false })) so local listeners are
updated to active=false before unmount; keep the
activeToastIdsRef.current.clear() and setIgnoreMouseEvents.mutate call and add
the notification call after them.
Comment on lines
81
to
87
| return () => { | ||
| window.removeEventListener( | ||
| TOAST_INTERACTION_STATE_EVENT, | ||
| handleToastInteractionState, | ||
| ); | ||
| console.debug("FloatingButton component unmounting"); | ||
| }; |
There was a problem hiding this comment.
Clear pending debounce timeout during unmount cleanup.
Line 82 cleanup removes the event listener, but a pending leaveTimeoutRef can still fire afterward and run stale state/mutation logic.
💡 Suggested fix
return () => {
+ if (leaveTimeoutRef.current) {
+ clearTimeout(leaveTimeoutRef.current);
+ leaveTimeoutRef.current = null;
+ }
window.removeEventListener(
TOAST_INTERACTION_STATE_EVENT,
handleToastInteractionState,
);
console.debug("FloatingButton component unmounting");
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return () => { | |
| window.removeEventListener( | |
| TOAST_INTERACTION_STATE_EVENT, | |
| handleToastInteractionState, | |
| ); | |
| console.debug("FloatingButton component unmounting"); | |
| }; | |
| return () => { | |
| if (leaveTimeoutRef.current) { | |
| clearTimeout(leaveTimeoutRef.current); | |
| leaveTimeoutRef.current = null; | |
| } | |
| window.removeEventListener( | |
| TOAST_INTERACTION_STATE_EVENT, | |
| handleToastInteractionState, | |
| ); | |
| console.debug("FloatingButton component unmounting"); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/widget/pages/widget/components/FloatingButton.tsx`
around lines 81 - 87, The unmount cleanup in the FloatingButton component
currently removes the TOAST_INTERACTION_STATE_EVENT listener
(handleToastInteractionState) but doesn't clear a pending debounce timeout,
allowing leaveTimeoutRef to fire after unmount; update the cleanup returned by
the useEffect to check leaveTimeoutRef.current and call
clearTimeout(leaveTimeoutRef.current) and then set leaveTimeoutRef.current =
null (or undefined) so any pending timeout is cancelled and cannot run stale
state/mutation logic after unmount.
haritabh-z01
approved these changes
Feb 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary\n- add a dismiss (X) button to widget error toasts with a larger hit target\n- add localized dismiss aria-label text for en/es/ja\n- stabilize widget toast interaction by coordinating mouse pass-through with active toast lifecycle\n\n## Validation\n- pnpm -C apps/desktop exec eslint src/renderer/widget/components/WidgetToast.tsx src/renderer/widget/components/ToasterWrapper.tsx src/renderer/widget/hooks/useWidgetNotifications.tsx src/renderer/widget/pages/widget/components/FloatingButton.tsx\n- pnpm -C apps/desktop type:check
Summary by CodeRabbit
Release Notes
New Features
Internationalization