CODE_REVIEW_SUMMARY.md

@@ -1,134 +0,0 @@
-# Code Review and Improvements Summary
-
-## Overview
-This document summarizes the code review findings and improvements made to the WhspBrd chat application, with a focus on Kitty image rendering and scrolling functionality.
-
-## Issues Found and Fixed
-
-### 1. Missing Scrolling Functionality ✅ FIXED
-**Issue**: The TODO indicated that scrolling with J/K keys needed to be implemented.
-
-**Solution**:
-- Added `chatScrollOffset` variable to track scroll position
-- Implemented `scrollChatUp()` and `scrollChatDown()` functions
-- Added J/K keybindings for Vim-style scrolling
-- Disabled `Autoscroll` on chat view to enable manual scrolling
-- Reset scroll offset when switching contacts or sending messages
-
-### 2. Image Positioning with Scroll ✅ FIXED
-**Issue**: Images were positioned at absolute terminal coordinates without accounting for scroll offset.
-
-**Solution**:
-- Modified image row calculation to subtract scroll offset: `rowPosition = i*messageRowIncrement + messageRowOffset - chatScrollOffset`
-- Added visibility check to only render images that are within the visible viewport
-- This ensures images scroll correctly with the text content
-
-### 3. Code Quality Improvements ✅ FIXED
-
-#### 3.1 DRY Principle Violation
-**Issue**: Message rendering code was duplicated for user and contact messages.
-
-**Solution**: Refactored to eliminate duplication:
-```go
-// Determine sender and icon path once
-isUserMessage := !strings.EqualFold(msg.Sender, users[selectedUserIdx])
-var senderLabel string
-var iconPath string
-
-if isUserMessage {
-    senderLabel = Colors.Text(Colors.Base02) + "You (" + formattedTime + "):" + Colors.Reset
-    iconPath = userIconPath
-} else {
-    senderLabel = Colors.Text(Colors.Base05) + msg.Sender + " (" + formattedTime + "):" + Colors.Reset
-    iconPath = fmt.Sprintf(contactIconPathFmt, strings.ToLower(msg.Sender))
-}
-
-// Single rendering call
-fmt.Fprintf(v, "%s", "\t\t\t\t\t"+senderLabel+"\n\t\t\t\t\t"+string(decoded)+"\n\n")
-```
-
-#### 3.2 Performance Optimization
-**Issue**: `cell_size.GetTerminalCellSizePixels()` was called for every message in the loop.
-
-**Solution**: Moved the call outside the loop to call it once per update, with fallback values if it fails.
-
-#### 3.3 Bounds Checking
-**Issue**: User validation was done inside the loop for every message.
-
-**Solution**: Moved the check before the loop, returning early if no valid users exist.
-
-## Kitty Image Protocol Implementation Review
-
-### Correctness ✅
-The Kitty image protocol implementation is **correct**:
-
-1. **Cursor Management**: Properly saves (`\033[s`) and restores (`\033[u`) cursor position
-2. **Graphics Protocol**: Correctly uses:
-   - `\033_G` to start graphics command
-   - `q=2` for suppressed response (performance optimization)
-   - `a=T` for transmit and display
-   - `f=32` for RGBA format
-   - `s=%d,v=%d` for image dimensions
-   - `m=1` for chunk continuation
-3. **Image Cleanup**: Uses `clean_image` package to clear images by column before redrawing
-4. **Image Resizing**: Properly handles aspect ratio with the resize package
-
-### Image Size Calculation
-The code uses terminal cell pixel dimensions to calculate image sizes:
-```go
-if h > w {
-    h = h * 2
-    w = 0  // Maintains aspect ratio
-} else {
-    w = w*3 - (w / 10)
-    h = 0  // Maintains aspect ratio
-}
-```
-This is intentional - setting one dimension to 0 tells the resize function to maintain aspect ratio.
-
-## Files Modified
-
-1. **internal/tui/tui.go**: Added `chatScrollOffset` variable
-2. **internal/tui/layout.go**: Disabled autoscroll on chat view
-3. **internal/tui/chat.go**: 
-   - Implemented scrolling functions
-   - Fixed image positioning for scroll
-   - Refactored for better code quality
-   - Added visibility checking for images
-4. **internal/tui/keybindings.go**: Added J/K keybindings for scrolling
-5. **internal/tui/sidebar.go**: Reset scroll offset when switching contacts
-6. **docs/TODO.md**: Updated to mark completed tasks
-
-## Testing Recommendations
-
-Since this is a TUI application with Kitty terminal graphics protocol, manual testing is recommended:
-
-1. **Test scrolling**: Use J/K keys to scroll through messages
-2. **Test with images**: Verify images scroll correctly with text
-3. **Test contact switching**: Ensure scroll resets when switching contacts
-4. **Test sending messages**: Verify scroll resets to bottom after sending
-5. **Test edge cases**: 
-   - Empty message history
-   - Single message
-   - Messages that exceed viewport height
-
-## Potential Future Improvements
-
-1. **Scroll by page**: Add Page Up/Page Down support for faster navigation
-2. **Smooth scrolling**: Consider multi-line scroll increments
-3. **Scroll to message**: Add ability to jump to specific messages
-4. **Scroll indicators**: Show scroll position indicator
-5. **Smart scroll bounds**: Calculate actual content height for better scroll limits
-6. **Image loading**: Add error handling for missing image files
-7. **Multiline messages**: Handle messages that wrap across multiple lines
-
-## Conclusion
-
-The code is now properly implemented with:
-- ✅ Working scrolling functionality (J/K keys)
-- ✅ Correct Kitty image rendering
-- ✅ Images that scroll properly with text content
-- ✅ Improved code quality and performance
-- ✅ Better maintainability through DRY principles
-
-No critical bugs or logical errors were found in the image rendering implementation. The Kitty protocol usage is correct and follows best practices.

docs/TODO.md

@@ -12,13 +12,13 @@
 
 ### Chat
 
-- [x] Add scrolling capability (J/K keys)
+- [ ] Add scrolling capability
 - [ ] Solve multiline messages and resizing the window
-- [x] Implement calculating positions of images and profile pictures
+- [ ] Implement calculating positions of images and profile pictures
-- [x] Add rendering image previews
+- [ ] Add rendering image previews
-  - [x] Render image in chat
+  - [ ] Render image in chat
-  - [x] Render image in chat with scroll
+  - [ ] Render image in chat with scroll
-- [x] Add rendering profile pictures
+- [ ] Add rendering profile pictures
 - [ ] Solve too long message history problem - render only first 100 messages?
 - [ ] Check chat without images, if kitty image protocol is not used
 - [ ] Check chat on windows

internal/tui/chat.go

@@ -28,58 +28,7 @@ func updateChatView(v *gocui.View) {
 	clear := cleanimage.NewKittyImageCleaner()
 	fmt.Print(clear.DeleteByColumn(chatViewColumn, false))
 
-	// Check if we have valid users
+	for i, msg := range chatData.Messages {
-	if len(users) == 0 || selectedUserIdx >= len(users) {
-		return
-	}
-
-	// Ensure selectedMessageIdx is within bounds
-	if len(chatData.Messages) == 0 {
-		return
-	}
-	if selectedMessageIdx < 0 {
-		selectedMessageIdx = 0
-	}
-	if selectedMessageIdx >= len(chatData.Messages) {
-		selectedMessageIdx = len(chatData.Messages) - 1
-	}
-
-	// Get view dimensions
-	_, viewHeight := v.Size()
-
-	// Calculate which messages should be visible
-	// We want to show the selected message in the middle of the view when possible
-	messagesPerView := viewHeight / messageRowIncrement
-	if messagesPerView <= 0 {
-		messagesPerView = 1
-	}
-
-	// Calculate start index - show newest messages at bottom
-	startIdx := max(0, min(selectedMessageIdx-(messagesPerView/2), len(chatData.Messages)-messagesPerView))
-
-	// Calculate scroll offset to position view correctly
-	scrollOffset := startIdx * messageRowIncrement
-	ox, _ := v.Origin()
-	v.SetOrigin(ox, scrollOffset)
-
-	// Get terminal cell size once for all messages
-	w, h, err := cell_size.GetTerminalCellSizePixels()
-	if err != nil {
-		log.Println("Error getting terminal cell size:", err)
-		w, h = 10, 20 // fallback values
-	}
-	if h > w {
-		h = h * 2
-		w = 0
-	} else {
-		w = w*3 - (w / 10)
-		h = 0
-	}
-
-	// Render only visible messages
-	endIdx := min(startIdx+messagesPerView+1, len(chatData.Messages))
-	for i := startIdx; i < endIdx; i++ {
-		msg := chatData.Messages[i]
 		decoded, err := base64.StdEncoding.DecodeString(msg.Content)
 		if err != nil {
 			log.Printf("Error decoding message: %v", err)
@@ -94,37 +43,30 @@ func updateChatView(v *gocui.View) {
 		}
 		formattedTime := t.Format("2006-01-02 15:04")
 
-		// Determine if this is a message from the user or the contact
+		w, h, err := cell_size.GetTerminalCellSizePixels()
-		isUserMessage := !strings.EqualFold(msg.Sender, users[selectedUserIdx])
+		if err != nil {
-		var senderLabel string
+			log.Println("Error getting terminal cell size:", err)
-		var iconPath string
+			continue
-
+		}
-		if isUserMessage {
+		if h > w {
-			senderLabel = Colors.Text(Colors.Base02) + "You (" + formattedTime + "):" + Colors.Reset
+			h = h * 2
-			iconPath = userIconPath
+			w = 0
 		} else {
-			senderLabel = Colors.Text(Colors.Base05) + msg.Sender + " (" + formattedTime + "):" + Colors.Reset
+			w = w*3 - (w / 10)
-			iconPath = fmt.Sprintf(contactIconPathFmt, strings.ToLower(msg.Sender))
+			h = 0
 		}
 
-		// Highlight selected message
+		if len(users) == 0 || selectedUserIdx >= len(users) {
-		prefix := "\t\t\t\t\t"
+			continue
-		if i == selectedMessageIdx {
-			prefix = "\x1b[7m\t\t\t\t\t"
-			senderLabel = senderLabel + "\x1b[0m\x1b[7m"
-			decoded = append([]byte("\x1b[0m\x1b[7m"), decoded...)
-			decoded = append(decoded, []byte("\x1b[0m")...)
 		}
 
-		// Print message text
+		if !strings.EqualFold(msg.Sender, users[selectedUserIdx]) {
-		fmt.Fprintf(v, "%s", prefix+senderLabel+"\n"+prefix+string(decoded)+"\n\n")
+			fmt.Fprintf(v, "%s", "\t\t\t\t\t"+Colors.Text(Colors.Base02)+"You ("+formattedTime+"):"+Colors.Reset+"\n\t\t\t\t\t"+string(decoded)+"\n\n")
-
+			render_image.RenderImage(userIconPath, i*messageRowIncrement+messageRowOffset, chatViewColumn, w, h, false)
-		// Calculate row position for image rendering
+		} else {
-		rowPosition := i*messageRowIncrement + messageRowOffset - scrollOffset
+			fmt.Fprintf(v, "%s", "\t\t\t\t\t"+Colors.Text(Colors.Base05)+msg.Sender+" ("+formattedTime+"):"+Colors.Reset+"\n\t\t\t\t\t"+string(decoded)+"\n\n")
-
+			iconPath := fmt.Sprintf(contactIconPathFmt, strings.ToLower(msg.Sender))
-		// Only render images that are visible in the view
+			render_image.RenderImage(iconPath, i*messageRowIncrement+messageRowOffset, chatViewColumn, w, h, false)
-		if rowPosition >= 0 && rowPosition < viewHeight {
-			render_image.RenderImage(iconPath, rowPosition, chatViewColumn, w, h, false)
 		}
 	}
 }
@@ -150,47 +92,6 @@ func sendMessage(g *gocui.Gui, v *gocui.View) error {
 		log.Printf("Error getting chat view: %v", err)
 		return err
 	}
-
-	// Select newest message (at the end)
-	if len(chatData.Messages) > 0 {
-		selectedMessageIdx = len(chatData.Messages) - 1
-	}
 	updateChatView(chatView)
 	return nil
 }
-
-func scrollChatUp(g *gocui.Gui, v *gocui.View) error {
-	if len(chatData.Messages) == 0 {
-		return nil
-	}
-
-	chatView, err := g.View("chat")
-	if err != nil {
-		return err
-	}
-
-	// Move selection up (to older messages)
-	if selectedMessageIdx > 0 {
-		selectedMessageIdx--
-		updateChatView(chatView)
-	}
-	return nil
-}
-
-func scrollChatDown(g *gocui.Gui, v *gocui.View) error {
-	if len(chatData.Messages) == 0 {
-		return nil
-	}
-
-	chatView, err := g.View("chat")
-	if err != nil {
-		return err
-	}
-
-	// Move selection down (to newer messages)
-	if selectedMessageIdx < len(chatData.Messages)-1 {
-		selectedMessageIdx++
-		updateChatView(chatView)
-	}
-	return nil
-}

internal/tui/keybindings.go

@@ -14,18 +14,6 @@ func keybindings(g *gocui.Gui) error {
 	if err := g.SetKeybinding("", gocui.KeyCtrlK, gocui.ModNone, prevContact); err != nil {
 		return err
 	}
-	if err := g.SetKeybinding("", 'j', gocui.ModNone, scrollChatDown); err != nil {
-		return err
-	}
-	if err := g.SetKeybinding("", 'J', gocui.ModNone, scrollChatDown); err != nil {
-		return err
-	}
-	if err := g.SetKeybinding("", 'k', gocui.ModNone, scrollChatUp); err != nil {
-		return err
-	}
-	if err := g.SetKeybinding("", 'K', gocui.ModNone, scrollChatUp); err != nil {
-		return err
-	}
 	if err := g.SetKeybinding("", gocui.KeyTab, gocui.ModNone, toggleProfileView); err != nil {
 		return err
 	}

internal/tui/layout.go

@@ -44,7 +44,7 @@ func layoutChat(g *gocui.Gui, maxX, maxY int) error {
 		}
 		v.Title = " Chat "
 		v.Wrap = true
-		v.Autoscroll = false
+		v.Autoscroll = true
 		updateChatView(v)
 	}
 	return nil

internal/tui/sidebar.go

@@ -74,16 +74,6 @@ func nextContact(g *gocui.Gui, v *gocui.View) error {
 		selectedUserIdx = 0
 	}
 
-	// Load messages for new contact
-	LoadMessages(users[selectedUserIdx])
-
-	// Select newest message (scroll to bottom)
-	if len(chatData.Messages) > 0 {
-		selectedMessageIdx = len(chatData.Messages) - 1
-	} else {
-		selectedMessageIdx = 0
-	}
-
 	if err := updateContactsView(g); err != nil {
 		return err
 	}
@@ -106,16 +96,6 @@ func prevContact(g *gocui.Gui, v *gocui.View) error {
 		selectedUserIdx = len(users) - 1
 	}
 
-	// Load messages for new contact
-	LoadMessages(users[selectedUserIdx])
-
-	// Select newest message (scroll to bottom)
-	if len(chatData.Messages) > 0 {
-		selectedMessageIdx = len(chatData.Messages) - 1
-	} else {
-		selectedMessageIdx = 0
-	}
-
 	if err := updateContactsView(g); err != nil {
 		return err
 	}

internal/tui/tui.go

@@ -10,7 +10,6 @@ var users []string
 var prevWidth, prevHeight int
 var chatData ChatData
 var selectedUserIdx int = 0
-var selectedMessageIdx int = 0
 
 func Run() {
 	LoadContacts(defaultServerPath)
@@ -18,10 +17,6 @@ func Run() {
 	// Load initial messages if there are any contacts
 	if len(users) > 0 && selectedUserIdx < len(users) {
 		LoadMessages(users[selectedUserIdx])
-		// Initialize to newest message (bottom)
-		if len(chatData.Messages) > 0 {
-			selectedMessageIdx = len(chatData.Messages) - 1
-		}
 	}
 
 	g, err := gocui.NewGui(gocui.OutputNormal)