Skip to content

feat (desktop): toggle for notifications #1045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AviPeltz merged 2 commits into from
Jan 29, 2026
Merged

feat (desktop): toggle for notifications #1045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e408be6
toggle for notifications
AviPeltz Jan 29, 2026
0696170
get rid of dead code
AviPeltz Jan 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions apps/desktop/src/lib/trpc/routers/settings/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,5 +385,25 @@ export const createSettingsRouter = () => {
authorPrefix: authorName?.toLowerCase().replace(/\s+/g, "-") ?? null,
};
}),

getNotificationSoundsMuted: publicProcedure.query(() => {
const row = getSettings();
return row.notificationSoundsMuted ?? false;
}),

setNotificationSoundsMuted: publicProcedure
.input(z.object({ muted: z.boolean() }))
.mutation(({ input }) => {
localDb
.insert(settings)
.values({ id: 1, notificationSoundsMuted: input.muted })
.onConflictDoUpdate({
target: settings.id,
set: { notificationSoundsMuted: input.muted },
})
.run();

return { success: true };
}),
});
};
19 changes: 18 additions & 1 deletion apps/desktop/src/main/lib/notification-sound.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,18 @@ import {
import { localDb } from "./local-db";
import { getSoundPath } from "./sound-paths";

/**
* Checks if notification sounds are muted.
*/
function areNotificationSoundsMuted(): boolean {
try {
const settingsRow = localDb.select().from(settings).get();
return settingsRow?.notificationSoundsMuted ?? false;
} catch {
return false;
}
}
Comment on lines +14 to +21
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jan 29, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Log the error before returning the default value.

The catch block silently swallows errors. Per coding guidelines, errors should at minimum be logged with context, even when providing a safe fallback.

Proposed fix 
 function areNotificationSoundsMuted(): boolean {
 	try {
 		const settingsRow = localDb.select().from(settings).get();
 		return settingsRow?.notificationSoundsMuted ?? false;
-	} catch {
+	} catch (error) {
+		console.error("[notification-sound/areNotificationSoundsMuted] Failed to read muted state:", error);
 		return false;
 	}
 }
🤖 Prompt for AI Agents 
In `@apps/desktop/src/main/lib/notification-sound.ts` around lines 14 - 21, The
catch block in areNotificationSoundsMuted currently swallows errors; update it
to log the caught error (include the error object and context indicating the
function and that a default false will be returned) before returning false—use
the project's logger if available (e.g., appLogger.error or similar), otherwise
console.error; reference localDb and settings in the log message so the failure
source is clear.


/**
* Gets the selected ringtone filename from the database.
* Falls back to default ringtone if the stored ID is invalid/stale.
Expand All @@ -19,7 +31,7 @@ function getSelectedRingtoneFilename(): string {
const settingsRow = localDb.select().from(settings).get();
const selectedId = settingsRow?.selectedRingtoneId ?? DEFAULT_RINGTONE_ID;

// "none" means silent - return empty string intentionally
// Legacy: "none" was previously used before the muted toggle existed
if (selectedId === "none") {
return "";
}
Expand Down Expand Up @@ -63,6 +75,11 @@ function playSoundFile(soundPath: string): void {
* Uses platform-specific commands to play the audio file.
*/
export function playNotificationSound(): void {
// Check if sounds are muted
if (areNotificationSoundsMuted()) {
return;
}

const filename = getSelectedRingtoneFilename();

// No sound if "none" is selected
Expand Down
106 changes: 58 additions & 48 deletions ...utes/_authenticated/settings/ringtones/components/RingtonesSettings/RingtonesSettings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { Label } from "@superset/ui/label";
import { Switch } from "@superset/ui/switch";
import { cn } from "@superset/ui/utils";
import { useCallback, useEffect, useRef, useState } from "react";
import { HiBellSlash, HiCheck, HiPlay, HiStop } from "react-icons/hi2";
import { HiCheck, HiPlay, HiStop } from "react-icons/hi2";
import { electronTrpc } from "renderer/lib/electron-trpc";
import { electronTrpcClient } from "renderer/lib/trpc-client";
import {
AVAILABLE_RINGTONES,
Expand Down Expand Up @@ -33,50 +36,6 @@ function RingtoneCard({
onSelect,
onTogglePlay,
}: RingtoneCardProps) {
const isSilent = ringtone.id === "none";

// Silent card has a distinct style
if (isSilent) {
return (
<button
type="button"
onClick={onSelect}
className={cn(
"relative flex flex-col rounded-lg border-2 overflow-hidden transition-all text-left",
isSelected
? "border-primary ring-2 ring-primary/20"
: "border-dashed border-border hover:border-muted-foreground/50",
)}
>
{/* Preview area */}
<div
className={cn(
"h-24 flex flex-col items-center justify-center relative gap-1",
isSelected ? "bg-accent/50" : "bg-muted/20",
)}
>
<HiBellSlash className="h-8 w-8 text-muted-foreground" />
<span className="text-xs text-muted-foreground">No sound</span>
</div>

{/* Info */}
<div className="p-3 bg-card border-t flex items-center justify-between">
<div>
<div className="text-sm font-medium">{ringtone.name}</div>
<div className="text-xs text-muted-foreground">
{ringtone.description}
</div>
</div>
{isSelected && (
<div className="h-5 w-5 rounded-full bg-primary flex items-center justify-center">
<HiCheck className="h-3 w-3 text-primary-foreground" />
</div>
)}
</div>
</button>
);
}

return (
// biome-ignore lint/a11y/useSemanticElements: Using div with role="button" to allow nested play/stop button
<div
Expand Down Expand Up @@ -169,6 +128,34 @@ export function RingtonesSettings({ visibleItems }: RingtonesSettingsProps) {
const [playingId, setPlayingId] = useState<string | null>(null);
const previewTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);

const utils = electronTrpc.useUtils();
const { data: isMutedData, isLoading: isMutedLoading } =
electronTrpc.settings.getNotificationSoundsMuted.useQuery();
const isMuted = isMutedData ?? false;

const setMuted = electronTrpc.settings.setNotificationSoundsMuted.useMutation(
{
onMutate: async ({ muted }) => {
await utils.settings.getNotificationSoundsMuted.cancel();
const previous = utils.settings.getNotificationSoundsMuted.getData();
utils.settings.getNotificationSoundsMuted.setData(undefined, muted);
return { previous };
},
onError: (_err, _vars, context) => {
if (context?.previous !== undefined) {
utils.settings.getNotificationSoundsMuted.setData(
undefined,
context.previous,
);
}
Comment on lines +146 to +152
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jan 29, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Log the error in onError handler.

The error is being silently handled without logging. Per coding guidelines, errors should at minimum be logged with context.

Proposed fix 
-		onError: (_err, _vars, context) => {
+		onError: (err, _vars, context) => {
+			console.error("[settings/notification-sounds] Failed to toggle muted state:", err);
 			if (context?.previous !== undefined) {
 				utils.settings.getNotificationSoundsMuted.setData(
 					undefined,
 					context.previous,
 				);
 			}
 		},
📝 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
onError: (_err, _vars, context) => {
if (context?.previous !== undefined) {
utils.settings.getNotificationSoundsMuted.setData(
undefined,
context.previous,
);
}
onError: (err, _vars, context) => {
console.error("[settings/notification-sounds] Failed to toggle muted state:", err);
if (context?.previous !== undefined) {
utils.settings.getNotificationSoundsMuted.setData(
undefined,
context.previous,
);
}
🤖 Prompt for AI Agents 
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/ringtones/components/RingtonesSettings/RingtonesSettings.tsx`
around lines 146 - 152, The onError handler for the mutation currently swallows
errors; update the onError in RingtonesSettings (the function using onError:
(_err, _vars, context) => { ... }) to log the error and context before or after
restoring state with
utils.settings.getNotificationSoundsMuted.setData(undefined, context.previous);
include the _err and context (or context.previous) in the log call (e.g.,
console.error or the app logger) so the failure is recorded with relevant
context.

},
},
);

const handleMutedToggle = (enabled: boolean) => {
setMuted.mutate({ muted: !enabled });
};

// Clean up timer and stop any playing sound on unmount
useEffect(() => {
return () => {
Expand All @@ -184,7 +171,7 @@ export function RingtonesSettings({ visibleItems }: RingtonesSettingsProps) {

const handleTogglePlay = useCallback(
async (ringtone: Ringtone) => {
if (ringtone.id === "none" || !ringtone.filename) {
if (!ringtone.filename) {
return;
}

Expand Down Expand Up @@ -251,8 +238,31 @@ export function RingtonesSettings({ visibleItems }: RingtonesSettingsProps) {
</div>

<div className="space-y-8">
{/* Ringtone Section */}
{/* Sound Toggle */}
{showNotification && (
<div className="flex items-center justify-between">
<div className="space-y-0.5">
<Label
htmlFor="notification-sounds"
className="text-sm font-medium"
>
Notification sounds
</Label>
<p className="text-xs text-muted-foreground">
Play a sound when tasks complete
</p>
</div>
<Switch
id="notification-sounds"
checked={!isMuted}
onCheckedChange={handleMutedToggle}
disabled={isMutedLoading || setMuted.isPending}
/>
</div>
)}

{/* Ringtone Section */}
{showNotification && !isMuted && (
<div>
<h3 className="text-sm font-medium mb-4">Notification Sound</h3>
<div className="grid grid-cols-2 lg:grid-cols-3 gap-4">
Expand All @@ -271,7 +281,7 @@ export function RingtonesSettings({ visibleItems }: RingtonesSettingsProps) {
)}

{/* Tip */}
{showNotification && (
{showNotification && !isMuted && (
<div className="pt-6 border-t">
<p className="text-sm text-muted-foreground">
Click the play button to preview a sound. Click stop or play
Expand Down
10 changes: 1 addition & 9 deletions apps/desktop/src/shared/ringtones.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,6 @@ export const RINGTONES: RingtoneData[] = [
color: "from-indigo-400 to-purple-600",
duration: 7,
},
{
id: "none",
name: "Silent",
description: "Notifications without sound",
filename: "",
emoji: "🔇",
color: "from-gray-400 to-gray-500",
},
];

export const DEFAULT_RINGTONE_ID = "arcade";
Expand All @@ -139,7 +131,7 @@ export function getRingtoneById(id: string): RingtoneData | undefined {

/**
* Get the filename for a ringtone ID.
* Returns empty string for "none" (silent) or if not found.
* Returns empty string if not found.
*/
export function getRingtoneFilename(id: string): string {
const ringtone = getRingtoneById(id);
Expand Down
1 change: 1 addition & 0 deletions packages/local-db/drizzle/0015_add_notification_sounds_muted.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE `settings` ADD `notification_sounds_muted` integer;
Loading