Skip to content

Conversation

@lyzno1
Copy link
Collaborator

@lyzno1 lyzno1 commented Aug 9, 2025

What & Why

What: Fix inconsistent highlight mutual exclusion behavior across sidebar navigation buttons by ensuring all buttons properly clear previous selections.

Why: Some sidebar buttons (History, Settings) were missing selectItem() calls, causing dual highlighting where multiple buttons could be highlighted simultaneously, creating visual confusion and inconsistent user experience.

Fixes #197

Pre-PR Checklist

Run these:

  • pnpm type-check
  • pnpm format:check
  • pnpm lint
  • pnpm build
  • pnpm i18n:check (if applicable)

Type

  • 🐛 Bug fix

Technical Details

Core Problem

The sidebar had inconsistent state management patterns:

Working correctly (with selectItem() calls):

  • Chat list items: selectItem('chat', chatId, true)
  • Favorite Apps: selectItem('app', appId)
  • New Chat: selectItem('chat', null, true)

Broken (missing selectItem() calls):

  • Chat History: Only router.push('/chat/history')
  • Settings: Only router.push('/settings')

This caused dual highlighting where previous selections persisted when navigating to History or Settings.

Fix Applied

Added consistent selectItem(null, null) calls to clear previous selections:

History Button (sidebar-header.tsx):

onClick={() => {
  selectItem(null, null); // 🎯 Clear previous selection
  router.push('/chat/history');
}}

Settings Button (sidebar-footer.tsx):

onClick={() => {
  selectItem(null, null); // 🎯 Clear previous selection  
  router.push('/settings');
}}

Unified Pattern

All sidebar buttons now follow consistent state management:

Selection Buttons (set specific state):

  • selectItem('chat', chatId, true) - Chat items
  • selectItem('app', appId) - Apps

Navigation Buttons (clear state):

  • selectItem('chat', null, true) - New Chat
  • selectItem(null, null) - History, Settings

Test Coverage

Added comprehensive test suite validating:

  • ✅ Consistent state management patterns
  • ✅ Mutual exclusion across all buttons
  • ✅ No dual highlighting scenarios
  • ✅ Performance characteristics (<2ms updates)

Zero Breaking Changes

  • ✅ All existing functionality preserved
  • ✅ No API changes
  • ✅ No new dependencies
  • ✅ Full backward compatibility
  • ✅ Type safety maintained

This fix ensures a consistent, predictable sidebar navigation experience with proper mutual exclusion behavior across all navigation components.

…avigation buttons

- Add selectItem(null, null) calls to History and Settings buttons to clear previous selections
- Maintain consistent state management pattern across all sidebar navigation components
- Fix dual highlighting issue where multiple buttons could be highlighted simultaneously
- Add comprehensive test coverage validating mutual exclusion behavior

All sidebar buttons now follow unified pattern:
- Selection buttons: selectItem(type, id) to set specific selection
- Navigation buttons: selectItem(null, null) to clear previous selections

Fixes #197
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. area:ui UI components, layouts, styling, accessibility. type:bug Something is not working; user-visible defect. labels Aug 9, 2025
@dosubot dosubot bot added the lgtm Looks good to me; approved by a reviewer. label Aug 9, 2025
@zhangxuhe1 zhangxuhe1 merged commit 00d11a5 into main Aug 9, 2025
15 checks passed
@lyzno1 lyzno1 deleted the fix/sidebar-highlight-mutex-comprehensive branch August 9, 2025 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ui UI components, layouts, styling, accessibility. lgtm Looks good to me; approved by a reviewer. size:L This PR changes 100-499 lines, ignoring generated files. type:bug Something is not working; user-visible defect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 Sidebar navigation buttons inconsistent highlight mutex behavior

3 participants