Make notifications more reliable

singiamtel · singiamtel · commit 243e754d1f24 · 2024-04-04T03:29:07.000+02:00
diff --git a/src/UI/components/RoomListComponent.tsx b/src/UI/components/RoomListComponent.tsx
@@ -15,14 +15,17 @@ const customIcons = {
 
 
 export function RoomListComponent(
-    { name, ID, notifications: { unread, mentions }, type }: Readonly<{
+    { name, ID, type }: Readonly<{
         name: string;
         ID: string;
         type: string;
-        notifications: { unread: number; mentions: number };
     }>,
 ) {
     const { setRoom } = useClientContext();
+    const notifications = useClientStore(state => state.notifications);
+    const currentNotifications = notifications[ID];
+    const unread = currentNotifications?.unread ?? 0;
+    const mentions = currentNotifications?.mentions ?? 0;
     const room = useClientStore(state => state.currentRoom);
     const customIcon = customIcons[ID as keyof typeof customIcons];
     const icon = customIcon ?? (type === 'battle' ?
@@ -48,7 +51,7 @@ export function RoomListComponent(
                 {/** Notification circle if it applies */}
                 {(unread > 0) ?
                     (
-                        <span className="rounded-full bg-white text-white text-xs p-1 h-1 w-1  absolute top-1/2  transform -translate-x-1/2 -translate-y-1/2" />
+                        <span className="rounded-full bg-gray-600 dark:bg-gray-600 text-xs p-1 h-1 w-1  absolute top-1/2  transform -translate-x-1/2 -translate-y-1/2" />
                     ) :
                     null}
                 <button
diff --git a/src/UI/components/single/Chat.tsx b/src/UI/components/single/Chat.tsx
@@ -187,8 +187,8 @@ export function MessageComponent(
     }
     return (
         <div
-            className={'pt-0.5 ' +
-        (hld ? 'bg-yellow-hl-body-light dark:bg-yellow-hl-body' : '')}
+            className={cn('px-1',
+                (hld ? 'bg-yellow-hl-body-light dark:bg-yellow-hl-body' : ''))}
         >
             <span className="text-gray-125 text-xs">
                 {time ? HHMMSS(time) : ''}
diff --git a/src/UI/components/single/ClientContext.tsx b/src/UI/components/single/ClientContext.tsx
@@ -1,7 +1,6 @@
 import { assertNever } from '@/lib/utils';
 import { Client, client, useClientStore } from '../../../client/client';
 import { Message } from '../../../client/message';
-import { notificationsEngine, RoomNotification } from '../../../client/notifications';
 import { Room } from '../../../client/room/room';
 import { loadCustomColors } from '../../../utils/namecolour';
 import { createContext, useCallback, useContext, useEffect, useMemo, useState } from 'react';
@@ -13,14 +12,12 @@ interface ClientContextType {
     setRoom:(room: string | 1 | -1 | Room) => void;
     messages: Message[];
     setRooms: (rooms: Room[]) => void;
-    notifications: RoomNotification[];
 }
 
 const ClientContext = createContext<ClientContextType | undefined>(undefined);
 
 export default function ClientContextProvider(props: Readonly<React.PropsWithChildren>) {
     const [rooms, setRooms] = useState<Room[]>(client.getRooms());
-    const [notifications, setNotifications] = useState<RoomNotification[]>([]);
     const [update, setUpdate] = useState<number>(0); // Used to force update on rooms change
     const [previousRooms, setPreviousRooms] = useState<string[]>(['home']);
     const [messages, setMessages] = useState<Message[]>([]);
@@ -133,7 +130,6 @@ export default function ClientContextProvider(props: Readonly<React.PropsWithChi
             return;
         }
         const msgs = currentRoom.messages ?? [];
-        setNotifications(client.getNotifications());
         setMessages([...msgs]);
     }, [client]);
 
@@ -146,20 +142,11 @@ export default function ClientContextProvider(props: Readonly<React.PropsWithChi
             setUpdateMsgs(updateMsgs + 1);
         };
 
-        const notificationsEventListener: EventListener = (event) => {
-            notificationsEngine.sendNotification((event as CustomEvent).detail);
-        };
-
         client.events.addEventListener('message', eventListener);
-        client.events.addEventListener('notification', notificationsEventListener);
 
         return () => {
             // Clean up the event listener when the component unmounts
             client.events.removeEventListener('message', eventListener);
-            client.events.removeEventListener(
-                'notification',
-                notificationsEventListener,
-            );
         };
     }, [
         client,
@@ -191,8 +178,7 @@ export default function ClientContextProvider(props: Readonly<React.PropsWithChi
         messages,
         rooms,
         setRooms,
-        notifications,
-    }), [messages, rooms, notifications]);
+    }), [messages, rooms, setRooms]);
 
     return (
         <ClientContext.Provider
diff --git a/src/UI/components/single/Sidebar.tsx b/src/UI/components/single/Sidebar.tsx
@@ -25,7 +25,7 @@ import { cn } from '@/lib/utils';
 import { useClientStore } from '@/client/client';
 
 export default function Sidebar(props: Readonly<HTMLAttributes<'div'>>) {
-    const rooms = useClientStore((state) => state.rooms);
+    const { rooms } = useClientStore((state) => ({ rooms: state.rooms }));
     const roomsArray = Array.from(rooms.values()).filter(room => room.open);
 
     const mouseSensor = useSensor(MouseSensor, {
@@ -84,10 +84,6 @@ export default function Sidebar(props: Readonly<HTMLAttributes<'div'>>) {
                                         name={room.name}
                                         type={room.type}
                                         ID={room.ID}
-                                        notifications={{
-                                            unread: room.unread,
-                                            mentions: room.mentions,
-                                        }}
                                     />
                                 </SortableItem>
                             ))}
diff --git a/src/UI/components/single/UserPanel.tsx b/src/UI/components/single/UserPanel.tsx
@@ -33,7 +33,6 @@ function RenderUserContent() {
 
     useEffect(() => {
         const onDisconnect = () => {
-            console.log('disconnected');
             setConnected(false);
         };
         client.events.addEventListener('disconnect', onDisconnect);
@@ -42,9 +41,6 @@ function RenderUserContent() {
         };
     }, []);
 
-    useEffect(() => {
-        console.log('connected', connected);
-    }, [connected]);
     if (!connected) {
         return <Disconnected />;
     } else if (user) {
diff --git a/src/client/client.ts b/src/client/client.ts
@@ -3,7 +3,7 @@ import { toID } from '../utils/generic';
 import newMessage, { Message } from './message';
 import { Room } from './room/room';
 import { User } from './user';
-import { clientNotification, RoomNotification } from './notifications';
+import { clientNotification, notificationsEngine, RoomNotification as RoomNotifications } from './notifications';
 import { Protocol } from '@pkmn/protocol';
 import { assert, assertNever } from '@/lib/utils';
 import { BattleRoom } from './room/battleRoom';
@@ -24,8 +24,10 @@ interface UseClientStoreType {
     // messages: Message[]; // only messages from the current room
     messages: Record<Room['ID'], Message[]>; // messages from all rooms
     newMessage: (room: Room, message: Message) => void;
-    notifications: RoomNotification[];
-    // setNotifications: (notifications: RoomNotification[]) => void;
+    notifications: Record<Room['ID'], RoomNotifications>;
+    clearNotifications: (roomID: Room['ID']) => void;
+    addUnread: (room: Room) => void;
+    addMention: (room: Room) => void;
     avatar: string;
     theme: 'light' | 'dark';
     user: string | undefined
@@ -42,6 +44,10 @@ export const useClientStore = create<UseClientStoreType>((set) => ({
     messages: {},
     newMessage: (room: Room, message: Message) => {
         set((state) => {
+            if (room !== state.currentRoom) {
+                state.addUnread(room);
+            }
+
             if (!state.messages[room.ID]) {
                 return {
                     messages: { ...state.messages, [room.ID]: [message] },
@@ -52,10 +58,48 @@ export const useClientStore = create<UseClientStoreType>((set) => ({
             };
         });
     },
-    notifications: [],
+    notifications: {},
+    addUnread: (room: Room) => {
+        set((state) => {
+            if (!state.notifications[room.ID]) {
+                return {
+                    notifications: { ...state.notifications, [room.ID]: { unread: 1, mentions: 0 } },
+                };
+            }
+            return {
+                notifications: { ...state.notifications, [room.ID]: { unread: state.notifications[room.ID].unread + 1, mentions: state.notifications[room.ID].mentions } },
+            };
+        });
+    },
+    addMention: (room: Room) => {
+        set((state) => {
+            if (!state.notifications[room.ID]) {
+                return {
+                    notifications: { ...state.notifications, [room.ID]: { unread: 0, mentions: 1 } },
+                };
+            }
+            return {
+                notifications: { ...state.notifications, [room.ID]: { unread: state.notifications[room.ID].unread, mentions: state.notifications[room.ID].mentions + 1 } },
+            };
+        });
+    },
+    clearNotifications: (roomID: string) => {
+        set((state) => {
+            if (!state.notifications[roomID]) {
+                return {
+                    notifications: { ...state.notifications, [roomID]: { unread: 0, mentions: 0 } },
+                };
+            }
+            return {
+                notifications: { ...state.notifications, [roomID]: { unread: 0, mentions: 0 } },
+            };
+        });
+    },
+
     avatar: 'lucas',
     theme: localStorage.getItem('theme') as 'light' | 'dark' ?? 'dark',
     user: undefined,
+
 }));
 
 
@@ -108,7 +152,7 @@ export class Client {
     constructor(options?: ClientConstructor) {
         // if running test suite, don't do anything
         if (import.meta.env.VITEST) {
-            console.log('Running tests, skipping client initialization');
+            console.debug('Running tests, skipping client initialization');
             return;
         }
         try {
@@ -211,8 +255,9 @@ export class Client {
     selectRoom(roomid: string) {
         this.__selectedRoom = roomid;
         this.room(roomid)?.select();
-        this.settings.changeRooms(this.rooms);
+        // this.settings.changeRooms(this.rooms);
         useClientStore.setState({ currentRoom: this.room(roomid) });
+        useClientStore.getState().clearNotifications(roomid);
     }
 
     async queryUser(user: string, callback: (json: any) => void) {
@@ -330,16 +375,29 @@ export class Client {
         return highlight;
     }
 
+    private shouldNotify(room: Room, message: Message) {
+        if (this.selectedRoom == room.ID && document.hasFocus()) return false;
+        if (room.checkMessageStaleness(message)) return false;
+        if (message.hld || room.type === 'pm') return true;
+        return false;
+    }
+
     private forceHighlightMsg(roomid: string, message: Message) {
         return this.highlightMsg(roomid, message, true);
     }
 
-    getNotifications(): RoomNotification[] {
-        return Array.from(this.rooms).map(([_, room]) => ({
-            room: room.ID,
-            mentions: room.mentions,
-            unread: room.unread,
-        }));
+    getNotifications(): Map<string, RoomNotifications> {
+        return new Map(
+            [...this.rooms].map(([roomID, room]) => [roomID, { unread: room.unread, mentions: room.mentions }]),
+        );
+    }
+
+    clearNotifications(roomID: string) {
+        const room = this.room(roomID);
+        if (room) {
+            room.clearNotifications();
+            useClientStore.getState().clearNotifications(roomID);
+        }
     }
 
     openSettings() {
@@ -509,7 +567,9 @@ export class Client {
     private _addRoom(room: Room) {
         this.rooms.set(room.ID, room);
         useClientStore.setState({ rooms: new Map(this.rooms) });
-        this.selectRoom(room.ID);
+        if (!this.settings.rooms.find((r) => r.ID === room.ID)?.open) {
+            this.selectRoom(room.ID);
+        }
         if (room.type !== 'permanent' && !this.settings.rooms.find((r) => r.ID === room.ID)) {
             this.settings.addRoom(room);
         }
@@ -544,6 +604,7 @@ export class Client {
         message: Message,
     ) {
         const room = this.room(roomID);
+        this.highlightMsg(roomID, message);
         if (!room) {
             console.warn('addMessageToRoom: room (' + roomID + ') is unknown. Message:', message);
             return;
@@ -552,24 +613,21 @@ export class Client {
             selected: this.selectedRoom === roomID,
             selfSent: toID(this.settings.username) === toID(message.user),
         };
-        let shouldNotify = false;
         if (message.name) {
             room.addUHTML(message, settings);
         } else {
-            shouldNotify = room.addMessage(message, settings);
+            room.addMessage(message, settings);
         }
         useClientStore.getState().newMessage(room, message);
-        if (shouldNotify) {
-            this.events.dispatchEvent(
-                new CustomEvent('notification', {
-                    detail: {
-                        user: message.user,
-                        message: message.content,
-                        room: roomID,
-                        roomType: room.type,
-                    } as clientNotification,
-                }),
-            );
+
+        if (this.shouldNotify(room, message)) {
+            notificationsEngine.sendNotification({
+                user: message.user ?? '',
+                message: message.content,
+                room: roomID,
+                roomType: room.type,
+            });
+            useClientStore.getState().addMention(room);
         }
     }
 
diff --git a/src/client/message.ts b/src/client/message.ts
@@ -17,7 +17,6 @@ export type Message = {
     type: MessageType;
     user?: string;
     timestamp?: Date;
-    notify: boolean;
     /** true if message is highlighted, false if not, null if not parsed yet */
     hld: boolean | null
     name?: string; // only defined for uhtml messages
@@ -29,9 +28,8 @@ export default function ({
     user,
     timestamp,
     hld,
-    notify,
     name,
-}: Omit<Optional<Message, 'hld' | 'notify'>, 'timestamp'> & {
+}: Omit<Optional<Message, 'hld'>, 'timestamp'> & {
     timestamp?: string;
 }): Message {
     if (type === 'uhtmlchange') {
@@ -44,6 +42,5 @@ export default function ({
         timestamp: timestamp ? new Date(Number(timestamp) * 1000) : undefined,
         hld: hld ?? null,
         name,
-        notify: notify ?? false,
     };
 }
diff --git a/src/client/notifications.ts b/src/client/notifications.ts
@@ -1,7 +1,6 @@
 import { toast } from '@/components/ui/use-toast';
 
 export type RoomNotification = {
-    room: string;
     mentions: number;
     unread: number;
 };
@@ -26,7 +25,6 @@ class NotificationsEngine {
             Notification.requestPermission().then((permission) => {
                 this.permission = permission;
             });
-            return;
         }
     }
 
@@ -37,13 +35,11 @@ class NotificationsEngine {
         if (document.hasFocus()) {
             notification.user = notification.user.trim();
             if (selectedRoom !== notification.room) {
-                // Toasts don't have a title so we merge everything into the message
-                // const message = notification.roomType === 'pm' ?
-                //     `PM from ${notification.user}: ${notification.message}` :
-                //     `${notification.room} - ${notification.user}: ${notification.message}`;
-                // toast(limitString(message, 150)); //TODO: Move to UI
+                const title = notification.roomType === 'pm' ?
+                    `PM from ${notification.user}` :
+                    `${notification.room} - ${notification.user}`;
                 toast({
-                    title: 'PM from ' + notification.user,
+                    title,
                     description: limitString(notification.message, 150),
                 });
             }
diff --git a/src/client/room/battleRoom.ts b/src/client/room/battleRoom.ts
diff --git a/src/client/room/room.ts b/src/client/room/room.ts
diff --git a/src/client/settings.ts b/src/client/settings.ts

Original file line number	Diff line number	Diff line change
`@@ -187,8 +187,8 @@ export function MessageComponent(`
`187`	`187`	`}`
`188`	`188`	`return (`
`189`	`189`	`<div`
`190`		`- className={'pt-0.5 ' +`
`191`		`- (hld ? 'bg-yellow-hl-body-light dark:bg-yellow-hl-body' : '')}`
	`190`	`+ className={cn('px-1',`
	`191`	`+ (hld ? 'bg-yellow-hl-body-light dark:bg-yellow-hl-body' : ''))}`
`192`	`192`	`>`
`193`	`193`	`<span className="text-gray-125 text-xs">`
`194`	`194`	`{time ? HHMMSS(time) : ''}`