Fix notification and sound system issues with comprehensive improvements

- **Fixed duplicate sound events**: Prevent success/shutdown sounds from playing multiple times during timeline switches and app quit - **Added event coordination patterns**: Implemented flags and source tracking to prevent circular UI event chains - **Completed sound event coverage**: Added missing sound methods (play_favorite, play_follow, play_unfollow, play_direct_message, play_post) and wired them to user actions - **Enhanced notification system**: Fixed desktop notifications ignoring enable/disable settings by adding missing notifications section to default config - **Improved timeline-specific sounds**: Conversations now use direct_message sound instead of generic timeline_update sound - **Added duplicate code prevention guidelines**: Comprehensive CLAUDE.md section with patterns, checklist, and testing requirements to prevent future duplicate implementations - **Sound pack compatibility**: Full support for both default pack (private_message, post_sent) and Doom pack (direct_message, post) naming conventions with intelligent fallback All sound events now properly trigger on user actions: boost/favorite posts, follow/unfollow users, notifications, timeline updates, and lifecycle events. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
2025-07-21 07:53:36 -04:00
parent 014f288524
commit 3c3b50bdb9
6 changed files with 158 additions and 10 deletions
@@ -7,6 +7,74 @@ Bifrost is a fully accessible fediverse client built with PySide6, designed spec
 - Check for any changes in git project before doing anything else. Make sure the latest changes have been pulled
 - See what has changed, use git commands and examine the code to make sure you are up to date with the latest code

+## Duplicate Code Prevention Guidelines
+
+### Critical Areas Requiring Attention
+1. **Sound/Audio Events**: Never add multiple paths that trigger the same sound event
+2. **UI Event Handlers**: Avoid circular event chains (A triggers B which triggers A)
+3. **Timeline Operations**: Coordinate refresh calls and state changes to prevent conflicts
+4. **Lifecycle Events**: Ensure shutdown, close, and quit events don't overlap
+
+### Required Patterns for Event-Heavy Operations
+
+#### Event Coordination Pattern
+```python
+def operation_method(self):
+    if hasattr(self, '_operation_in_progress') and self._operation_in_progress:
+        return
+    self._operation_in_progress = True
+    try:
+        # perform operation
+        self.sound_manager.play_success()
+    finally:
+        # Reset flag after brief delay to prevent rapid-fire calls
+        QTimer.singleShot(100, lambda: setattr(self, '_operation_in_progress', False))
+```
+
+#### Event Source Tracking Pattern
+```python
+def ui_triggered_method(self, index, from_source="unknown"):
+    # Handle differently based on source: "tab_change", "keyboard", "menu"
+    if from_source == "tab_change":
+        # Skip certain UI updates to prevent circular calls
+        pass
+```
+
+#### Lifecycle Event Coordination Pattern
+```python
+def quit_application(self):
+    self._shutdown_handled = True  # Mark that shutdown is being handled
+    self.sound_manager.play_shutdown()
+    
+def closeEvent(self, event):
+    # Only handle if not already handled by explicit quit
+    if not hasattr(self, '_shutdown_handled'):
+        self.sound_manager.play_shutdown()
+```
+
+### Development Review Checklist
+Before implementing any sound, notification, or UI event:
+1. **Is there already another code path that triggers this same feedback?**
+2. **Could this create an event loop (A calls B which calls A)?**
+3. **Can rapid user actions (keyboard shortcuts) trigger this multiple times?**
+4. **Does this operation need coordination with other similar operations?**
+5. **Are there multiple UI elements (menu + button + shortcut) that trigger this?**
+
+### Common Patterns to Avoid
+- Multiple event handlers calling the same sound method
+- UI updates that trigger their own event handlers
+- Shutdown/close/quit events without coordination
+- Timeline refresh without checking if already refreshing
+- Direct sound calls in multiple places for the same user action
+
+### Testing Requirements
+When adding event-driven features, always test:
+- Rapid keyboard shortcut usage
+- Multiple quick UI interactions  
+- Combinations of keyboard shortcuts + UI clicks
+- Window closing during operations
+- Tab switching with keyboard vs mouse
+
 ## Documentation and Dependencies
 - **README Updates**: When adding new functionality or sound events, update README.md with detailed descriptions
 - **Requirements Management**: Check and update requirements.txt when new dependencies are added
@@ -86,10 +86,15 @@ class SoundManager:
    # Standard sound events
    SOUND_EVENTS = [
        "private_message",
+        "direct_message",
        "mention", 
        "boost",
        "reply",
+        "favorite",
+        "follow",
+        "unfollow",
        "post_sent",
+        "post",
        "timeline_update",
        "notification",
        "startup",
@@ -97,7 +102,9 @@ class SoundManager:
        "success",
        "error",
        "expand",
-        "collapse"
+        "collapse",
+        "autocomplete",
+        "autocomplete_end"
    ]
    
    def __init__(self, settings: SettingsManager):
@@ -142,10 +149,15 @@ class SoundManager:
                "version": "1.0",
                "sounds": {
                    "private_message": "private_message.wav",
+                    "direct_message": "direct_message.wav",
                    "mention": "mention.wav",
                    "boost": "boost.wav", 
                    "reply": "reply.wav",
+                    "favorite": "favorite.wav",
+                    "follow": "follow.wav",
+                    "unfollow": "unfollow.wav",
                    "post_sent": "post_sent.wav",
+                    "post": "post.wav",
                    "timeline_update": "timeline_update.wav",
                    "notification": "notification.wav",
                    "startup": "startup.wav",
@@ -154,7 +166,8 @@ class SoundManager:
                    "error": "error.wav",
                    "expand": "expand.wav",
                    "collapse": "collapse.wav",
-                    "autocomplete": "autocomplete.wav"
+                    "autocomplete": "autocomplete.wav",
+                    "autocomplete_end": "autocomplete_end.wav"
                }
            }
            
@@ -345,6 +358,10 @@ class SoundManager:
        """Play private message sound"""
        self.play_event("private_message")
        
+    def play_direct_message(self):
+        """Play direct message sound"""
+        self.play_event("direct_message")
+        
    def play_mention(self):
        """Play mention sound"""
        self.play_event("mention")
@@ -357,10 +374,26 @@ class SoundManager:
        """Play reply sound"""
        self.play_event("reply")
        
+    def play_favorite(self):
+        """Play favorite sound"""
+        self.play_event("favorite")
+        
+    def play_follow(self):
+        """Play follow sound"""
+        self.play_event("follow")
+        
+    def play_unfollow(self):
+        """Play unfollow sound"""
+        self.play_event("unfollow")
+        
    def play_post_sent(self):
        """Play post sent sound"""
        self.play_event("post_sent")
        
+    def play_post(self):
+        """Play post sound"""
+        self.play_event("post")
+        
    def play_timeline_update(self):
        """Play timeline update sound"""
        self.play_event("timeline_update")
@@ -94,6 +94,16 @@ class SettingsManager:
        self.config.set('interface', 'show_timestamps', 'true')
        self.config.set('interface', 'compact_mode', 'false')
        
+        # Desktop notification settings
+        self.config.add_section('notifications')
+        self.config.set('notifications', 'enabled', 'true')
+        self.config.set('notifications', 'direct_messages', 'true')
+        self.config.set('notifications', 'mentions', 'true')
+        self.config.set('notifications', 'boosts', 'false')
+        self.config.set('notifications', 'favorites', 'false')
+        self.config.set('notifications', 'follows', 'true')
+        self.config.set('notifications', 'timeline_updates', 'false')
+        
        self.save_settings()
        
    def save_settings(self):
@@ -446,9 +446,9 @@ class MainWindow(QMainWindow):
        
    def on_timeline_tab_changed(self, index):
        """Handle timeline tab change"""
-        self.switch_timeline(index)
+        self.switch_timeline(index, from_tab_change=True)
        
-    def switch_timeline(self, index):
+    def switch_timeline(self, index, from_tab_change=False):
        """Switch to timeline by index with loading feedback"""
        timeline_names = ["Home", "Messages", "Mentions", "Local", "Federated", "Bookmarks", "Followers", "Following"]
        timeline_types = ["home", "conversations", "notifications", "local", "federated", "bookmarks", "followers", "following"]
@@ -457,8 +457,13 @@ class MainWindow(QMainWindow):
            timeline_name = timeline_names[index]
            timeline_type = timeline_types[index]
            
-            # Set tab to match if called from keyboard shortcut
-            if self.timeline_tabs.currentIndex() != index:
+            # Prevent duplicate calls for the same timeline
+            if hasattr(self, '_current_timeline_switching') and self._current_timeline_switching:
+                return
+            self._current_timeline_switching = True
+            
+            # Set tab to match if called from keyboard shortcut (but not if already from tab change)
+            if not from_tab_change and self.timeline_tabs.currentIndex() != index:
                self.timeline_tabs.setCurrentIndex(index)
            
            # Announce loading
@@ -478,6 +483,10 @@ class MainWindow(QMainWindow):
                if hasattr(self.timeline, 'sound_manager'):
                    self.timeline.sound_manager.play_error()
                self.status_bar.showMessage(f"Failed to load {timeline_name} timeline: {str(e)}", 3000)
+            finally:
+                # Reset the flag after a brief delay to allow the operation to complete
+                from PySide6.QtCore import QTimer
+                QTimer.singleShot(100, lambda: setattr(self, '_current_timeline_switching', False))
                
    def get_selected_post(self):
        """Get the currently selected post from timeline"""
@@ -591,6 +600,9 @@ class MainWindow(QMainWindow):
            else:
                client.reblog_status(post.id)
                self.status_bar.showMessage("Post boosted", 2000)
+                # Play boost sound for successful boost
+                if hasattr(self.timeline, 'sound_manager'):
+                    self.timeline.sound_manager.play_boost()
            # Refresh timeline to show updated state
            self.timeline.refresh()
        except Exception as e:
@@ -610,6 +622,9 @@ class MainWindow(QMainWindow):
            else:
                client.favourite_status(post.id)
                self.status_bar.showMessage("Post favorited", 2000)
+                # Play favorite sound for successful favorite
+                if hasattr(self.timeline, 'sound_manager'):
+                    self.timeline.sound_manager.play_favorite()
            # Refresh timeline to show updated state
            self.timeline.refresh()
        except Exception as e:
@@ -630,6 +645,7 @@ class MainWindow(QMainWindow):
            
    def quit_application(self):
        """Quit the application with shutdown sound"""
+        self._shutdown_sound_played = True  # Mark that we're handling the shutdown sound
        if hasattr(self.timeline, 'sound_manager'):
            self.timeline.sound_manager.play_shutdown()
            # Wait briefly for sound to start playing
@@ -761,6 +777,9 @@ class MainWindow(QMainWindow):
            client.follow_account(post.account.id)
            username = post.account.display_name or post.account.username
            self.status_bar.showMessage(f"Followed {username}", 2000)
+            # Play follow sound for successful follow
+            if hasattr(self.timeline, 'sound_manager'):
+                self.timeline.sound_manager.play_follow()
        except Exception as e:
            self.status_bar.showMessage(f"Follow failed: {str(e)}", 3000)
            
@@ -776,6 +795,9 @@ class MainWindow(QMainWindow):
            client.unfollow_account(post.account.id)
            username = post.account.display_name or post.account.username
            self.status_bar.showMessage(f"Unfollowed {username}", 2000)
+            # Play unfollow sound for successful unfollow
+            if hasattr(self.timeline, 'sound_manager'):
+                self.timeline.sound_manager.play_unfollow()
        except Exception as e:
            self.status_bar.showMessage(f"Unfollow failed: {str(e)}", 3000)
            
@@ -858,14 +880,17 @@ class MainWindow(QMainWindow):
            client.follow_account(target_account['id'])
            display_name = target_account.get('display_name') or target_account['username']
            self.status_bar.showMessage(f"Followed {display_name}", 2000)
+            # Play follow sound for successful follow
+            if hasattr(self.timeline, 'sound_manager'):
+                self.timeline.sound_manager.play_follow()
            
        except Exception as e:
            self.status_bar.showMessage(f"Follow failed: {str(e)}", 3000)

    def closeEvent(self, event):
        """Handle window close event"""
-        # Play shutdown sound if not already played through quit_application
-        if hasattr(self.timeline, 'sound_manager'):
+        # Only play shutdown sound if not already played through quit_application
+        if not hasattr(self, '_shutdown_sound_played') and hasattr(self.timeline, 'sound_manager'):
            self.timeline.sound_manager.play_shutdown()
            # Wait briefly for sound to complete
            from PySide6.QtCore import QTimer, QEventLoop
@@ -301,7 +301,7 @@ class ComposeDialog(QDialog):
        }
        
        # Play sound when post button is pressed
-        self.sound_manager.play_event("post")
+        self.sound_manager.play_post()
        
        # Emit signal with all post data for background processing
        self.post_sent.emit(post_data)
@@ -177,14 +177,18 @@ class TimelineView(AccessibleTreeWidget):
                            
                            if notification_type == 'mention':
                                self.notification_manager.notify_mention(sender, content_preview)
+                                self.sound_manager.play_mention()
                            elif notification_type == 'reblog':
                                self.notification_manager.notify_boost(sender, content_preview)
+                                self.sound_manager.play_boost()
                            elif notification_type == 'favourite':
                                self.notification_manager.notify_favorite(sender, content_preview)
+                                self.sound_manager.play_favorite()
                    elif notification_type == 'follow':
                        # Handle follow notifications without status (skip if initial load)
                        if not self.skip_notifications:
                            self.notification_manager.notify_follow(sender)
+                            self.sound_manager.play_follow()
                except Exception as e:
                    print(f"Error parsing notification: {e}")
                    continue
@@ -285,11 +289,19 @@ class TimelineView(AccessibleTreeWidget):
                timeline_name = {
                    'home': 'home timeline',
                    'local': 'local timeline', 
-                    'federated': 'federated timeline'
+                    'federated': 'federated timeline',
+                    'conversations': 'conversations'
                }.get(self.timeline_type, 'timeline')
                
                # Use generic "new content" message instead of counting posts
                self.notification_manager.notify_new_content(timeline_name)
+                # Play appropriate sound based on timeline type
+                if self.timeline_type == 'conversations':
+                    # Use direct message sound for conversation updates
+                    self.sound_manager.play_direct_message()
+                else:
+                    # Use timeline update sound for other timelines
+                    self.sound_manager.play_timeline_update()
                
        # Build thread structure (accounts don't need threading)
        if self.timeline_type in ["followers", "following"]: