fix: key repeat support and TextInput selection bugs by kcjonson · Pull Request #30 · kcjonson/worldsim

kcjonson · 2025-11-26T02:43:43Z

Summary

Enable key repeat (GLFW_REPEAT) in application key input callback
Fix mouse release tracking bug that prevented focus changes
Add focus check to drag selection to prevent ghost selections in unfocused fields
Remove excessive per-frame logging for better performance
Add TextInput demo scene for testing and demonstration

Bugs Fixed

Key repeat not working: Keys would not repeat when held down (Delete, Backspace, arrow keys, etc.)
- Root cause: Application's key input callback was filtering out GLFW_REPEAT actions
- Fix: Modified filter to accept both GLFW_PRESS and GLFW_REPEAT
Right-to-left selection persisting in unfocused fields: When selecting text right-to-left and then clicking another field, the selection would persist in the unfocused field
- Root cause Implement string hashing system with FNV-1a algorithm #1: Mouse release code was inside the IsMouseButtonDown block, so m_mouseDown never cleared
- Root cause Fix logging system: Ensure all logs reach developer client #2: Drag selection didn't check if field was focused
- Fix: Moved mouse release code outside the block and added focus check to drag selection

Test plan

Verify keys repeat when held down (Delete, Backspace, arrows)
Verify right-to-left selection clears when switching fields
Verify left-to-right selection works correctly
Verify focus changes work properly after multiple clicks
Verify performance improvement from reduced logging

… support This commit implements the first phase of the TextInput component along with a comprehensive focus management system for keyboard navigation. New Features: - FocusManager: Centralized keyboard focus handling with Tab navigation - IFocusable interface: Type-erased interface for focusable UI components - TextInput Phase 1: Core text editing with cursor, keyboard navigation, and mouse input - UTF-8 utilities: Character boundary navigation for multi-byte text support Components Added: - libs/ui/focus/: FocusManager and IFocusable interface with unit tests - libs/ui/components/text_input/: TextInput component (Phase 1 complete, Phase 2 stubbed) - libs/foundation/utils/utf8: UTF-8 encoding/decoding and boundary navigation Integration: - InputManager: Integrated Tab key handling for focus navigation - Application: Added FocusManager lifecycle management - Button: Refactored to implement IFocusable interface Phase 1 Features: - Single-line text input with UTF-8 support - Cursor positioning with blinking animation - Insert/delete characters at cursor - Keyboard navigation (arrows, home, end, backspace, delete) - Mouse click to position cursor - Placeholder text support - Horizontal scrolling for overflow text - Character validation callback - Focus integration for Tab navigation Documentation: - Technical specs for focus management and text input design

Implements text selection and clipboard operations for the TextInput component. Features added: - Mouse drag selection: Click and drag to select text - Keyboard selection: Shift+Arrow keys to extend selection - Clipboard operations: Ctrl+C (copy), Ctrl+X (cut), Ctrl+V (paste), Ctrl+A (select all) - Visual selection rendering with configurable highlight color - Selection deletion: Delete/Backspace removes selected text - Typing replaces selection - Selection cleared on focus loss or navigation without Shift Implementation details: - Uses GLFW clipboard API (glfwGetClipboardString/glfwSetClipboardString) - Respects UTF-8 character boundaries in all selection operations - Filters newlines and control characters on paste - Hides cursor when selection is active - Selection rendered below text with semi-transparent overlay Modified: - libs/ui/components/text_input/text_input.cpp: - Implemented all Phase 2 selection methods (SetSelection, ClearSelection, etc.) - Implemented all Phase 2 clipboard methods (Copy, Cut, Paste, SelectAll) - Added mouse drag selection to HandleInput() - Added Shift+Arrow handling to HandleKeyInput() - Added Ctrl+C/X/V/A handling to HandleKeyInput() - Updated InsertChar, DeleteChar* to delete selection first - Implemented RenderSelection() to draw selection highlight - Updated RenderCursor() to hide when selection active - Updated OnFocusLost() to clear selection

Fixed critical bug where cursor jumped hundreds of pixels per character typed. Root cause was passing fontSize directly to MeasureText() instead of converting to scale (where 1.0 = 16px base size), making all measurements 16x too large. Changes: - Convert fontSize to scale before all MeasureText() calls in: - GetCursorXPosition() - GetCursorPositionFromMouse() - UpdateHorizontalScroll() - RenderSelection() - Refactor text rendering to use TextBatchRenderer directly instead of UI::Text component for precise positioning control needed for scrolling - Fix vertical text alignment to match Text component's bounding box Middle alignment: baselineY = position.y + (size.y - ascent) * 0.5F - Add text_batch_renderer.h include - Add debug logging for focus and input handling

- Enable key repeat (GLFW_REPEAT) in application key input callback - Fix mouse release tracking bug that prevented focus changes - Add focus check to drag selection to prevent ghost selections - Remove excessive per-frame logging for better performance - Add TextInput demo scene Fixes two critical bugs: 1. Keys would not repeat when held down (Delete, Backspace, arrows) 2. Right-to-left selection would persist in unfocused fields

Copilot

Pull request overview

This PR adds key repeat support for TextInput fields and fixes critical selection bugs by introducing a comprehensive focus management system with a new TextInput component.

Key Changes:

Implemented centralized FocusManager for keyboard focus tracking and Tab navigation
Added TextInput component with selection, clipboard, and UTF-8 support
Fixed key repeat issue by accepting GLFW_REPEAT in Application's input callback
Fixed mouse release tracking bug where m_mouseDown was never cleared due to being inside IsMouseButtonDown block

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
libs/ui/focus/focusable.h	Defines IFocusable interface for components that can receive keyboard focus
libs/ui/focus/focus_manager.h	Declares FocusManager class for centralized focus state management
libs/ui/focus/focus_manager.cpp	Implements focus navigation, scope management, and input routing
libs/ui/focus/focus_manager.test.cpp	Comprehensive test suite for FocusManager (838 lines of tests)
libs/ui/components/text_input/text_input.h	Header for single-line text input component with selection and clipboard
libs/ui/components/text_input/text_input.cpp	Implementation of TextInput with cursor, selection, and UTF-8 handling
libs/ui/components/button/button.h	Updated Button to implement IFocusable for keyboard navigation
libs/ui/components/button/button.cpp	Button integration with FocusManager, keyboard activation support
libs/foundation/utils/utf8.h	UTF-8 utility functions for character boundary detection and encoding
libs/foundation/utils/utf8.cpp	Implementation of UTF-8 encode/decode and character navigation
libs/engine/input/input_manager.h	Added callback system for external focus management integration
libs/engine/input/input_manager.cpp	Implements key and char input callbacks, GLFW key conversion
libs/engine/application/application.h	Added FocusManager instance and accessor
libs/engine/application/application.cpp	Wires InputManager callbacks to FocusManager, handles Tab navigation
libs/ui/CMakeLists.txt	Added text_input.cpp and focus_manager.cpp to build
libs/foundation/CMakeLists.txt	Added utf8.cpp to build
docs/technical/ui-framework/text-input.md	Comprehensive documentation for TextInput component (1117 lines)
docs/technical/ui-framework/focus-management.md	Documentation for focus management system (621 lines)
apps/ui-sandbox/scenes/text_input_scene.cpp	Demo scene showcasing TextInput features and Tab navigation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-26T02:46:39Z

+		if (m_focusManager != nullptr) {
+			m_focusManager->UnregisterFocusable(&other);
+			m_focusManager->RegisterFocusable(this, -1);
+		}
+		other.m_focusManager = nullptr; // Prevent double-unregister
+	}
+
+	TextInput& TextInput::operator=(TextInput&& other) noexcept {
+		if (this != &other) {
+			// Unregister this from old FocusManager
+			if (m_focusManager != nullptr) {
+				m_focusManager->UnregisterFocusable(this);
+			}
+
+			// Move data
+			m_position = other.m_position;
+			m_size = other.m_size;
+			m_text = std::move(other.m_text);
+			m_placeholder = std::move(other.m_placeholder);
+			m_style = other.m_style;
+			m_onChange = std::move(other.m_onChange);
+			m_charValidator = std::move(other.m_charValidator);
+			m_cursorPosition = other.m_cursorPosition;
+			m_selection = other.m_selection;
+			m_cursorBlinkTimer = other.m_cursorBlinkTimer;
+			m_horizontalScroll = other.m_horizontalScroll;
+			zIndex = other.zIndex;
+			visible = other.visible;
+			id = other.id;
+			m_enabled = other.m_enabled;
+			m_focused = other.m_focused;
+			m_mouseDown = other.m_mouseDown;
+			m_focusManager = other.m_focusManager;
+
+			// Unregister other from its old address, register this at new address
+			if (m_focusManager != nullptr) {
+				m_focusManager->UnregisterFocusable(&other);
+				m_focusManager->RegisterFocusable(this, -1);
+			}


In the move constructor and move assignment operator, when re-registering with the FocusManager after unregistering the other, the original tabIndex is lost and replaced with auto-assign (-1). This means that moving a TextInput will change its tab order, which could break the intended navigation order in forms. The original tabIndex should be preserved during the move.

Copilot · 2025-11-26T02:46:39Z

+	Button::Button(Button&& other) noexcept
+		: m_position(other.m_position),
+		  m_size(other.m_size),
+		  m_label(std::move(other.m_label)),
+		  m_state(other.m_state),
+		  m_disabled(other.m_disabled),
+		  m_focused(other.m_focused),
+		  m_appearance(other.m_appearance),
+		  m_onClick(std::move(other.m_onClick)),
+		  zIndex(other.zIndex),
+		  visible(other.visible),
+		  id(other.id),
+		  m_mouseOver(other.m_mouseOver),
+		  m_mouseDown(other.m_mouseDown),
+		  m_labelText(other.m_labelText),
+		  m_focusManager(other.m_focusManager) {
+		// Unregister other from its old address, register this at new address
+		if (m_focusManager != nullptr) {
+			m_focusManager->UnregisterFocusable(&other);
+			// We need to register with the same tabIndex, but FocusManager doesn't store it separately
+			// For now, just register with auto-assign
+			m_focusManager->RegisterFocusable(this, -1);
+		}
+		other.m_focusManager = nullptr; // Prevent double-unregister
+	}
+
+	Button& Button::operator=(Button&& other) noexcept {
+		if (this != &other) {
+			// Unregister this from old FocusManager
+			if (m_focusManager != nullptr) {
+				m_focusManager->UnregisterFocusable(this);
+			}
+
+			// Move data
+			m_position = other.m_position;
+			m_size = other.m_size;
+			m_label = std::move(other.m_label);
+			m_state = other.m_state;
+			m_disabled = other.m_disabled;
+			m_focused = other.m_focused;
+			m_appearance = other.m_appearance;
+			m_onClick = std::move(other.m_onClick);
+			zIndex = other.zIndex;
+			visible = other.visible;
+			id = other.id;
+			m_mouseOver = other.m_mouseOver;
+			m_mouseDown = other.m_mouseDown;
+			m_labelText = other.m_labelText;
+			m_focusManager = other.m_focusManager;
+
+			// Unregister other from its old address, register this at new address
+			if (m_focusManager != nullptr) {
+				m_focusManager->UnregisterFocusable(&other);
+				m_focusManager->RegisterFocusable(this, -1);
+			}
+			other.m_focusManager = nullptr;
+		}
+		return *this;


Same issue as in TextInput: the move constructor and move assignment operator lose the original tabIndex and use auto-assign (-1) when re-registering with the FocusManager. This will change the button's position in the tab order after a move operation.

Copilot · 2025-11-26T02:46:39Z

+class FocusManager {
+  public:
+	FocusManager() = default;
+	~FocusManager() = default;
+
+	// Disable copy/move (singleton-like usage)
+	FocusManager(const FocusManager&) = delete;
+	FocusManager& operator=(const FocusManager&) = delete;
+	FocusManager(FocusManager&&) = delete;
+	FocusManager& operator=(FocusManager&&) = delete;


The FocusManager class is declared as non-copyable and non-movable (lines 42-45), but it appears to have a static instance accessor pattern based on usage in other files (e.g., UI::FocusManager::Get() in application.cpp line 26). However, there's no Get() method or static instance mechanism defined in this header file. This will cause compilation errors.

Copilot · 2025-11-26T02:46:40Z

+#include "components/text_input/text_input.h"
+#include "focus/focus_manager.h"
+#include "font/font_renderer.h"
+#include "font/text_batch_renderer.h"
+#include "shapes/shapes.h"
+#include "utils/utf8.h"
+#include <GLFW/glfw3.h>
+#include <algorithm>
+#include <input/input_manager.h>
+#include <primitives/primitives.h>
+#include <utils/log.h>
+
+namespace UI {
+
+	// ============================================================================
+	// Constructor / Destructor
+	// ============================================================================
+
+	TextInput::TextInput(const Args& args)
+		: m_position(args.position),
+		  m_size(args.size),
+		  m_text(args.text),
+		  m_placeholder(args.placeholder),
+		  m_style(args.style),
+		  m_onChange(args.onChange),
+		  m_charValidator(args.charValidator),
+		  zIndex(args.zIndex),
+		  id(args.id),
+		  m_enabled(args.enabled),
+		  m_focusManager(args.focusManager) {
+
+		LOG_INFO(UI, "TextInput(%s): Created with m_enabled=%d", id, m_enabled);
+
+		// Set cursor to end of text
+		m_cursorPosition = m_text.size();
+
+		// Register with FocusManager if provided
+		if (m_focusManager != nullptr) {
+			m_focusManager->RegisterFocusable(this, args.tabIndex);
+		}
+	}
+
+	TextInput::~TextInput() {
+		// Unregister from FocusManager if registered
+		if (m_focusManager != nullptr) {
+			m_focusManager->UnregisterFocusable(this);
+		}
+	}
+
+	TextInput::TextInput(TextInput&& other) noexcept
+		: m_position(other.m_position),
+		  m_size(other.m_size),
+		  m_text(std::move(other.m_text)),
+		  m_placeholder(std::move(other.m_placeholder)),
+		  m_style(other.m_style),
+		  m_onChange(std::move(other.m_onChange)),
+		  m_charValidator(std::move(other.m_charValidator)),
+		  m_cursorPosition(other.m_cursorPosition),
+		  m_selection(other.m_selection),
+		  m_cursorBlinkTimer(other.m_cursorBlinkTimer),
+		  m_horizontalScroll(other.m_horizontalScroll),
+		  zIndex(other.zIndex),
+		  visible(other.visible),
+		  id(other.id),
+		  m_enabled(other.m_enabled),
+		  m_focused(other.m_focused),
+		  m_focusManager(other.m_focusManager),
+		  m_mouseDown(other.m_mouseDown) {
+		// Unregister other from its old address, register this at new address
+		if (m_focusManager != nullptr) {
+			m_focusManager->UnregisterFocusable(&other);
+			m_focusManager->RegisterFocusable(this, -1);
+		}
+		other.m_focusManager = nullptr; // Prevent double-unregister
+	}
+
+	TextInput& TextInput::operator=(TextInput&& other) noexcept {
+		if (this != &other) {
+			// Unregister this from old FocusManager
+			if (m_focusManager != nullptr) {
+				m_focusManager->UnregisterFocusable(this);
+			}
+
+			// Move data
+			m_position = other.m_position;
+			m_size = other.m_size;
+			m_text = std::move(other.m_text);
+			m_placeholder = std::move(other.m_placeholder);
+			m_style = other.m_style;
+			m_onChange = std::move(other.m_onChange);
+			m_charValidator = std::move(other.m_charValidator);
+			m_cursorPosition = other.m_cursorPosition;
+			m_selection = other.m_selection;
+			m_cursorBlinkTimer = other.m_cursorBlinkTimer;
+			m_horizontalScroll = other.m_horizontalScroll;
+			zIndex = other.zIndex;
+			visible = other.visible;
+			id = other.id;
+			m_enabled = other.m_enabled;
+			m_focused = other.m_focused;
+			m_mouseDown = other.m_mouseDown;
+			m_focusManager = other.m_focusManager;
+
+			// Unregister other from its old address, register this at new address
+			if (m_focusManager != nullptr) {
+				m_focusManager->UnregisterFocusable(&other);
+				m_focusManager->RegisterFocusable(this, -1);
+			}
+			other.m_focusManager = nullptr;
+		}
+		return *this;
+	}
+
+	// ============================================================================
+	// Lifecycle Methods
+	// ============================================================================
+
+	void TextInput::HandleInput() {
+		if (!m_enabled) {
+			return;
+		}
+
+		auto& input = engine::InputManager::Get();
+
+		// Check for mouse click
+		// Use IsMouseButtonDown (not IsMouseButtonPressed) like Button does
+		if (input.IsMouseButtonDown(engine::MouseButton::Left)) {
+			bool wasMouseDown = m_mouseDown;
+			if (!wasMouseDown) {
+				// Mouse just pressed - process initial click
+				Foundation::Vec2 mousePos = input.GetMousePosition();
+				LOG_DEBUG(UI, "TextInput(%s): Mouse clicked at (%.1f, %.1f)", id, mousePos.x, mousePos.y);
+
+				// Check if click is inside text input
+				if (ContainsPoint(mousePos)) {
+					LOG_INFO(UI, "TextInput(%s): Click inside bounds, grabbing focus", id);
+					// Grab focus
+					if (m_focusManager != nullptr) {
+						m_focusManager->SetFocus(this);
+					}
+
+					// Position cursor from mouse X
+					float localX = mousePos.x - m_position.x - m_style.paddingLeft + m_horizontalScroll;
+					m_cursorPosition = GetCursorPositionFromMouse(localX);
+					m_cursorBlinkTimer = 0.0F;
+
+					// Clear selection on click (will be recreated if dragging)
+					ClearSelection();
+
+					// Track mouse down for drag selection
+					m_mouseDown = true;
+				}
+			}
+
+			// Mouse drag selection
+			if (m_focused && m_mouseDown && input.IsMouseButtonDown(engine::MouseButton::Left)) {
+				Foundation::Vec2 mousePos = input.GetMousePosition();
+				float			 localX = mousePos.x - m_position.x - m_style.paddingLeft + m_horizontalScroll;
+				size_t			 dragPosition = GetCursorPositionFromMouse(localX);
+
+				// Create/update selection from original cursor position to drag position
+				if (dragPosition != m_cursorPosition) {
+					// Selection anchor is where mouse was first pressed (already set as m_cursorPosition)
+					// Selection head is current drag position
+					if (!m_selection.has_value()) {
+						// Start new selection
+						SetSelection(m_cursorPosition, dragPosition);
+					} else {
+						// Update existing selection
+						m_selection->end = dragPosition;
+					}
+					m_cursorPosition = dragPosition; // Move cursor to drag position
+					UpdateHorizontalScroll();
+				}
+			}
+
+		}
+
+	// Release mouse (outside the IsMouseButtonDown block)
+	if (m_mouseDown && !input.IsMouseButtonDown(engine::MouseButton::Left)) {
+		m_mouseDown = false;
+	}
+	}
+
+	void TextInput::Update(float deltaTime) {
+		if (m_focused) {
+			m_cursorBlinkTimer += deltaTime;
+			if (m_cursorBlinkTimer > m_style.cursorBlinkRate) {
+				m_cursorBlinkTimer -= m_style.cursorBlinkRate;
+			}
+		}
+	}
+
+	void TextInput::Render() const {
+		if (!visible) {
+			return;
+		}
+
+		RenderBackground();
+
+		// Scissor test to clip overflowing text
+		float textAreaX = m_position.x + m_style.paddingLeft;
+		float textAreaY = m_position.y + m_style.paddingTop;
+		float textAreaWidth = m_size.x - m_style.paddingLeft - m_style.paddingRight;
+		float textAreaHeight = m_size.y - m_style.paddingTop - m_style.paddingBottom;
+
+		Renderer::Primitives::PushScissor(Foundation::Rect{textAreaX, textAreaY, textAreaWidth, textAreaHeight});
+
+		if (m_text.empty() && !m_focused) {
+			RenderPlaceholder();
+		} else {
+			RenderSelection(); // Phase 2 (currently stub)
+			RenderText();
+			RenderCursor();
+		}
+
+		Renderer::Primitives::PopScissor();
+	}
+
+	// ============================================================================
+	// State Management
+	// ============================================================================
+
+	void TextInput::SetText(const std::string& text) {
+		m_text = text;
+		m_cursorPosition = std::min(m_cursorPosition, m_text.size());
+		UpdateHorizontalScroll();
+		if (m_onChange) {
+			m_onChange(m_text);
+		}
+	}
+
+	// ============================================================================
+	// IFocusable Interface
+	// ============================================================================
+
+	void TextInput::OnFocusGained() {
+		LOG_INFO(UI, "TextInput(%s): Focus gained", id);
+		m_focused = true;
+		m_cursorBlinkTimer = 0.0F; // Reset blink (cursor visible)
+	}
+
+	void TextInput::OnFocusLost() {
+		LOG_INFO(UI, "TextInput(%s): Focus lost", id);
+		m_focused = false;
+		ClearSelection();
+	}
+
+	void TextInput::HandleKeyInput(engine::Key key, bool shift, bool ctrl, bool /*alt*/) {
+		if (!m_enabled || !m_focused) {
+			return;
+		}
+
+		// Clipboard operations (Ctrl+C, Ctrl+X, Ctrl+V, Ctrl+A)
+		if (ctrl) {
+			switch (key) {
+				case engine::Key::C:
+					Copy();
+					return;
+				case engine::Key::X:
+					Cut();
+					return;
+				case engine::Key::V:
+					Paste();
+					return;
+				case engine::Key::A:
+					SelectAll();
+					return;
+				default:
+					break;
+			}
+		}
+
+		// Navigation and editing
+		switch (key) {
+			case engine::Key::Left:
+				if (shift) {
+					ExtendSelectionLeft();
+				} else {
+					ClearSelection();
+					MoveCursorLeft();
+				}
+				break;
+			case engine::Key::Right:
+				if (shift) {
+					ExtendSelectionRight();
+				} else {
+					ClearSelection();
+					MoveCursorRight();
+				}
+				break;
+			case engine::Key::Home:
+				ClearSelection();
+				MoveCursorHome();
+				break;
+			case engine::Key::End:
+				ClearSelection();
+				MoveCursorEnd();
+				break;
+			case engine::Key::Delete:
+				DeleteCharAtCursor();
+				break;
+			case engine::Key::Backspace:
+				DeleteCharBeforeCursor();
+				break;
+			default:
+				break;
+		}
+	}
+
+	void TextInput::HandleCharInput(char32_t codepoint) {
+		if (!m_enabled || !m_focused) {
+			return;
+		}
+
+		LOG_DEBUG(UI, "TextInput(%s): Received char input: U+%04X", id, codepoint);
+
+		// Filter control characters (except Tab which may be used)
+		if (codepoint < 32 || codepoint == 127) {
+			return; // Skip control characters
+		}
+
+		InsertChar(codepoint);
+	}
+
+	bool TextInput::CanReceiveFocus() const {
+		return m_enabled;
+	}
+
+	// ============================================================================
+	// Phase 1: Core Editing Operations
+	// ============================================================================
+
+	void TextInput::InsertChar(char32_t codepoint) {
+		// Validate character (if validator provided)
+		if (m_charValidator && !m_charValidator(codepoint)) {
+			return; // Reject character
+		}
+
+		// Delete selection first if active
+		if (m_selection.has_value() && !m_selection->IsEmpty()) {
+			LOG_INFO(UI, "InsertChar: deleting selection before insert");
+			DeleteSelection();
+		} else {
+			LOG_INFO(UI, "InsertChar: no selection, inserting at cursor");
+		}
+
+		// Convert codepoint to UTF-8
+		std::string utf8 = foundation::UTF8::Encode(codepoint);
+
+		// Insert at cursor position
+		m_text.insert(m_cursorPosition, utf8);
+		m_cursorPosition += utf8.size();
+
+		// Reset cursor blink
+		m_cursorBlinkTimer = 0.0F;
+
+		// Update scroll and notify
+		UpdateHorizontalScroll();
+		if (m_onChange) {
+			m_onChange(m_text);
+		}
+	}
+
+	void TextInput::DeleteCharAtCursor() {
+		// Delete selection if active
+		if (m_selection.has_value() && !m_selection->IsEmpty()) {
+			DeleteSelection();
+			return;
+		}
+
+		if (m_cursorPosition >= m_text.size()) {
+			return; // At end
+		}
+
+		// Find next UTF-8 character boundary
+		size_t charSize = foundation::UTF8::CharacterSize(static_cast<unsigned char>(m_text[m_cursorPosition]));
+		m_text.erase(m_cursorPosition, charSize);
+
+		m_cursorBlinkTimer = 0.0F;
+		UpdateHorizontalScroll();
+		if (m_onChange) {
+			m_onChange(m_text);
+		}
+	}
+
+	void TextInput::DeleteCharBeforeCursor() {
+		// Delete selection if active
+		if (m_selection.has_value() && !m_selection->IsEmpty()) {
+			DeleteSelection();
+			return;
+		}
+
+		if (m_cursorPosition == 0) {
+			return; // At start
+		}
+
+		// Find previous UTF-8 character boundary
+		size_t charSize = foundation::UTF8::PreviousCharacterSize(m_text, m_cursorPosition);
+		m_text.erase(m_cursorPosition - charSize, charSize);
+		m_cursorPosition -= charSize;
+
+		m_cursorBlinkTimer = 0.0F;
+		UpdateHorizontalScroll();
+		if (m_onChange) {
+			m_onChange(m_text);
+		}
+	}
+
+	void TextInput::MoveCursorLeft() {
+		if (m_cursorPosition == 0) {
+			return;
+		}
+
+		size_t charSize = foundation::UTF8::PreviousCharacterSize(m_text, m_cursorPosition);
+		m_cursorPosition -= charSize;
+		m_cursorBlinkTimer = 0.0F;
+		UpdateHorizontalScroll();
+	}
+
+	void TextInput::MoveCursorRight() {
+		if (m_cursorPosition >= m_text.size()) {
+			return;
+		}
+
+		size_t charSize = foundation::UTF8::CharacterSize(static_cast<unsigned char>(m_text[m_cursorPosition]));
+		m_cursorPosition += charSize;
+		m_cursorBlinkTimer = 0.0F;
+		UpdateHorizontalScroll();
+	}
+
+	void TextInput::MoveCursorHome() {
+		m_cursorPosition = 0;
+		m_cursorBlinkTimer = 0.0F;
+		UpdateHorizontalScroll();
+	}
+
+	void TextInput::MoveCursorEnd() {
+		m_cursorPosition = m_text.size();
+		m_cursorBlinkTimer = 0.0F;
+		UpdateHorizontalScroll();
+	}
+
+	// ============================================================================
+	// Phase 2: Selection Operations
+	// ============================================================================
+
+	void TextInput::SetSelection(size_t start, size_t end) {
+		m_selection = TextSelection{start, end};
+		m_cursorBlinkTimer = 0.0F;
+		LOG_INFO(UI, "SetSelection(%zu, %zu)", start, end);
+	}
+
+	void TextInput::ClearSelection() {
+		if (m_selection.has_value()) {
+			LOG_INFO(UI, "ClearSelection (was %zu-%zu)", m_selection->start, m_selection->end);
+		}
+		m_selection.reset();
+	}
+
+	std::string TextInput::GetSelectedText() const {
+		if (!m_selection.has_value() || m_selection->IsEmpty()) {
+			return "";
+		}
+
+		size_t start = m_selection->GetMin();
+		size_t end = m_selection->GetMax();
+		return m_text.substr(start, end - start);
+	}
+
+	void TextInput::DeleteSelection() {
+		if (!m_selection.has_value() || m_selection->IsEmpty()) {
+			LOG_INFO(UI, "DeleteSelection called but no selection");
+			return;
+		}
+
+		size_t start = m_selection->GetMin();
+		size_t end = m_selection->GetMax();
+		LOG_INFO(UI, "DeleteSelection deleting range %zu-%zu", start, end);
+
+		// Delete the selected range
+		m_text.erase(start, end - start);
+
+		// Move cursor to start of deleted range
+		m_cursorPosition = start;
+
+		// Clear selection
+		ClearSelection();
+
+		// Update and notify
+		m_cursorBlinkTimer = 0.0F;
+		UpdateHorizontalScroll();
+		if (m_onChange) {
+			m_onChange(m_text);
+		}
+	}
+
+	void TextInput::ExtendSelectionLeft() {
+		if (m_cursorPosition == 0) {
+			return;
+		}
+
+		// If no selection exists, create one starting at current cursor
+		if (!m_selection.has_value()) {
+			m_selection = TextSelection{m_cursorPosition, m_cursorPosition};
+		}
+
+		// Move cursor left (selection end)
+		size_t charSize = foundation::UTF8::PreviousCharacterSize(m_text, m_cursorPosition);
+		m_cursorPosition -= charSize;
+		m_selection->end = m_cursorPosition;
+
+		m_cursorBlinkTimer = 0.0F;
+		UpdateHorizontalScroll();
+	}
+
+	void TextInput::ExtendSelectionRight() {
+		if (m_cursorPosition >= m_text.size()) {
+			return;
+		}
+
+		// If no selection exists, create one starting at current cursor
+		if (!m_selection.has_value()) {
+			m_selection = TextSelection{m_cursorPosition, m_cursorPosition};
+		}
+
+		// Move cursor right (selection end)
+		size_t charSize = foundation::UTF8::CharacterSize(static_cast<unsigned char>(m_text[m_cursorPosition]));
+		m_cursorPosition += charSize;
+		m_selection->end = m_cursorPosition;
+
+		m_cursorBlinkTimer = 0.0F;
+		UpdateHorizontalScroll();
+	}
+
+	// ============================================================================
+	// Phase 2: Clipboard Operations
+	// ============================================================================
+
+	void TextInput::Copy() {
+		std::string selectedText = GetSelectedText();
+		if (selectedText.empty()) {
+			return;
+		}
+
+		// Set clipboard using GLFW
+		GLFWwindow* window = glfwGetCurrentContext();
+		if (window != nullptr) {
+			glfwSetClipboardString(window, selectedText.c_str());
+		}
+	}
+
+	void TextInput::Cut() {
+		std::string selectedText = GetSelectedText();
+		if (selectedText.empty()) {
+			return;
+		}
+
+		// Copy to clipboard
+		GLFWwindow* window = glfwGetCurrentContext();
+		if (window != nullptr) {
+			glfwSetClipboardString(window, selectedText.c_str());
+		}
+
+		// Delete selection
+		DeleteSelection();
+	}
+
+	void TextInput::Paste() {
+		// Get clipboard text using GLFW
+		GLFWwindow* window = glfwGetCurrentContext();
+		if (window == nullptr) {
+			return;
+		}
+
+		const char* clipboardText = glfwGetClipboardString(window);
+		if (clipboardText == nullptr || clipboardText[0] == '\0') {
+			return;
+		}
+
+		// Delete selection if active
+		if (m_selection.has_value() && !m_selection->IsEmpty()) {
+			DeleteSelection();
+		}
+
+		// Insert clipboard text at cursor
+		// Convert to UTF-8 codepoints and insert each character
+		std::string			  pasteText(clipboardText);
+		std::vector<char32_t> codepoints = foundation::UTF8::Decode(pasteText);
+
+		for (char32_t codepoint : codepoints) {
+			// Filter newlines and control characters
+			if (codepoint == '\n' || codepoint == '\r' || (codepoint < 32 && codepoint != '\t')) {
+				continue;
+			}
+
+			// Validate character if validator provided
+			if (m_charValidator && !m_charValidator(codepoint)) {
+				continue;
+			}
+
+			// Insert character
+			std::string utf8 = foundation::UTF8::Encode(codepoint);
+			m_text.insert(m_cursorPosition, utf8);
+			m_cursorPosition += utf8.size();
+		}
+
+		// Update and notify
+		m_cursorBlinkTimer = 0.0F;
+		UpdateHorizontalScroll();
+		if (m_onChange) {
+			m_onChange(m_text);
+		}
+	}
+
+	void TextInput::SelectAll() {
+		if (m_text.empty()) {
+			return;
+		}
+
+		SetSelection(0, m_text.size());
+		m_cursorPosition = m_text.size(); // Cursor at end
+		UpdateHorizontalScroll();
+	}
+
+	// ============================================================================
+	// Rendering Helpers
+	// ============================================================================
+
+	void TextInput::RenderBackground() const {
+		Foundation::Color borderColor = m_focused ? m_style.focusedBorderColor : m_style.borderColor;
+
+		Foundation::RectStyle rectStyle{
+			.fill = m_style.backgroundColor,
+			.border = Foundation::BorderStyle{.color = borderColor, .width = m_style.borderWidth, .cornerRadius = m_style.cornerRadius}
+		};
+
+		Renderer::Primitives::DrawRect(
+			{.bounds = {m_position.x, m_position.y, m_size.x, m_size.y}, .style = rectStyle, .id = id, .zIndex = static_cast<int>(zIndex)}
+		);
+	}
+
+	void TextInput::RenderText() const {
+		if (m_text.empty()) {
+			return;
+		}
+
+		// Get batch renderer
+		ui::TextBatchRenderer* batchRenderer = Renderer::Primitives::GetTextBatchRenderer();
+		if (batchRenderer == nullptr) {
+			return;
+		}
+
+		// Get font renderer for metrics
+		ui::FontRenderer* fontRenderer = Renderer::Primitives::GetFontRenderer();
+		if (fontRenderer == nullptr) {
+			return;
+		}
+
+		// Convert fontSize to scale (16px base size = 1.0 scale)
+		constexpr float BASE_FONT_SIZE = 16.0F;
+		float			scale = m_style.fontSize / BASE_FONT_SIZE;
+
+		// Calculate horizontal position with scroll
+		float textX = m_position.x + m_style.paddingLeft - m_horizontalScroll;
+
+		// Calculate baseline Y position for vertically centered text
+		// This matches Text component's bounding box Middle alignment behavior
+		float ascent = fontRenderer->GetAscent(scale);
+		float baselineY = m_position.y + (m_size.y - ascent) * 0.5F;
+
+		// Add text to batch
+		batchRenderer->AddText(
+			m_text,
+			glm::vec2(textX, baselineY),
+			scale,
+			glm::vec4(m_style.textColor.r, m_style.textColor.g, m_style.textColor.b, m_style.textColor.a),
+			zIndex + 0.1F
+		);
+	}
+
+	void TextInput::RenderCursor() const {
+		if (!m_focused) {
+			return;
+		}
+
+		// Don't render cursor when selection is active
+		if (m_selection.has_value() && !m_selection->IsEmpty()) {
+			return;
+		}
+
+		// Blink: visible for first half of cycle
+		bool visible = m_cursorBlinkTimer < (m_style.cursorBlinkRate * 0.5F);
+		if (!visible) {
+			return;
+		}
+
+		float cursorX = GetCursorXPosition();
+		float centerY = m_position.y + (m_size.y * 0.5F); // Vertical center of input box
+		float textHeight = m_style.fontSize;
+
+		// Position cursor vertically centered with the text
+		float cursorStartY = centerY - (textHeight * 0.5F);
+		float cursorEndY = centerY + (textHeight * 0.5F);
+
+		Renderer::Primitives::DrawLine(
+			{.start = {cursorX, cursorStartY},
+			 .end = {cursorX, cursorEndY},
+			 .style = Foundation::LineStyle{.color = m_style.cursorColor, .width = m_style.cursorWidth},
+			 .id = id,
+			 .zIndex = static_cast<int>(zIndex + 0.2F)}
+		);
+	}
+
+	void TextInput::RenderSelection() const {
+		if (!m_selection.has_value() || m_selection->IsEmpty()) {
+			return;
+		}
+
+		size_t start = m_selection->GetMin();
+		size_t end = m_selection->GetMax();
+
+		// Get FontRenderer for text measurement
+		ui::FontRenderer* fontRenderer = Renderer::Primitives::GetFontRenderer();
+		if (fontRenderer == nullptr) {
+			return;
+		}
+
+		// Convert fontSize to scale (16px base size = 1.0 scale)
+		constexpr float BASE_FONT_SIZE = 16.0F;
+		float			scale = m_style.fontSize / BASE_FONT_SIZE;
+
+		// Measure text before selection start
+		std::string textBeforeStart = m_text.substr(0, start);
+		float		startX = fontRenderer->MeasureText(textBeforeStart, scale).x;
+
+		// Measure text up to selection end
+		std::string textBeforeEnd = m_text.substr(0, end);
+		float		endX = fontRenderer->MeasureText(textBeforeEnd, scale).x;
+
+		// Calculate selection rectangle (vertically centered in the box)
+		float selectionX = m_position.x + m_style.paddingLeft + startX - m_horizontalScroll;
+		float selectionWidth = endX - startX;
+		float centerY = m_position.y + (m_size.y * 0.5F);
+		float selectionHeight = m_style.fontSize;
+		float selectionY = centerY - (selectionHeight * 0.5F);
+
+		// Draw selection background
+		Foundation::RectStyle selectionStyle{.fill = m_style.selectionColor, .border = std::nullopt};
+
+		Renderer::Primitives::DrawRect({
+			.bounds = {selectionX, selectionY, selectionWidth, selectionHeight},
+			.style = selectionStyle,
+			.id = id,
+			.zIndex = static_cast<int>(zIndex + 0.05F) // Below text but above background
+		});
+	}
+
+	void TextInput::RenderPlaceholder() const {
+		if (m_placeholder.empty()) {
+			return;
+		}
+
+		// Get batch renderer
+		ui::TextBatchRenderer* batchRenderer = Renderer::Primitives::GetTextBatchRenderer();
+		if (batchRenderer == nullptr) {
+			return;
+		}
+
+		// Get font renderer for metrics
+		ui::FontRenderer* fontRenderer = Renderer::Primitives::GetFontRenderer();
+		if (fontRenderer == nullptr) {
+			return;
+		}
+
+		// Convert fontSize to scale (16px base size = 1.0 scale)
+		constexpr float BASE_FONT_SIZE = 16.0F;
+		float			scale = m_style.fontSize / BASE_FONT_SIZE;
+
+		// Calculate horizontal position (no scroll for placeholder)
+		float textX = m_position.x + m_style.paddingLeft;
+
+		// Calculate baseline Y position for vertically centered text
+		// This matches Text component's bounding box Middle alignment behavior
+		float ascent = fontRenderer->GetAscent(scale);
+		float baselineY = m_position.y + (m_size.y - ascent) * 0.5F;
+
+		// Add placeholder to batch
+		batchRenderer->AddText(
+			m_placeholder,
+			glm::vec2(textX, baselineY),
+			scale,
+			glm::vec4(m_style.placeholderColor.r, m_style.placeholderColor.g, m_style.placeholderColor.b, m_style.placeholderColor.a),
+			zIndex + 0.1F
+		);
+	}
+
+	// ============================================================================
+	// Utilities
+	// ============================================================================
+
+	float TextInput::GetCursorXPosition() const {
+		// Get text up to cursor position
+		std::string textBeforeCursor = m_text.substr(0, m_cursorPosition);
+
+		// Measure width using FontRenderer
+		// Convert fontSize to scale (16px base size = 1.0 scale)
+		constexpr float BASE_FONT_SIZE = 16.0F;
+		float			scale = m_style.fontSize / BASE_FONT_SIZE;
+
+		ui::FontRenderer* fontRenderer = Renderer::Primitives::GetFontRenderer();
+		float			  width = fontRenderer ? fontRenderer->MeasureText(textBeforeCursor, scale).x : 0.0F;
+
+		// Account for padding and scroll
+		return m_position.x + m_style.paddingLeft + width - m_horizontalScroll;
+	}
+
+	size_t TextInput::GetCursorPositionFromMouse(float localX) const {
+		// localX is already relative to text area (with scroll applied)
+
+		// Binary search for closest character boundary
+		size_t bestPosition = 0;
+		float  bestDistance = std::abs(localX);
+
+		// Convert fontSize to scale (16px base size = 1.0 scale)
+		constexpr float BASE_FONT_SIZE = 16.0F;
+		float			scale = m_style.fontSize / BASE_FONT_SIZE;
+
+		// Check each character boundary
+		ui::FontRenderer* fontRenderer = Renderer::Primitives::GetFontRenderer();
+		for (size_t i = 0; i <= m_text.size();) {
+			std::string textUpToPos = m_text.substr(0, i);
+			float		width = fontRenderer ? fontRenderer->MeasureText(textUpToPos, scale).x : 0.0F;
+			float		distance = std::abs(width - localX);
+
+			if (distance < bestDistance) {
+				bestDistance = distance;
+				bestPosition = i;
+			}
+
+			// Move to next character boundary
+			if (i >= m_text.size()) {
+				break;
+			}
+			size_t charSize = foundation::UTF8::CharacterSize(static_cast<unsigned char>(m_text[i]));
+			i += charSize;
+		}
+
+		return bestPosition;
+	}
+
+	void TextInput::UpdateHorizontalScroll() {
+		float cursorX = GetCursorXPosition() - m_position.x - m_style.paddingLeft + m_horizontalScroll;
+		float visibleWidth = m_size.x - m_style.paddingLeft - m_style.paddingRight;
+
+		// Scroll right if cursor is past right edge
+		if (cursorX > visibleWidth) {
+			m_horizontalScroll += (cursorX - visibleWidth);
+		}
+
+		// Scroll left if cursor is past left edge
+		if (cursorX < 0) {
+			m_horizontalScroll += cursorX; // cursorX is negative
+		}
+
+		// Clamp to ensure text fills from left when possible
+		// Convert fontSize to scale (16px base size = 1.0 scale)
+		constexpr float BASE_FONT_SIZE = 16.0F;
+		float			scale = m_style.fontSize / BASE_FONT_SIZE;
+
+		ui::FontRenderer* fontRenderer = Renderer::Primitives::GetFontRenderer();
+		float			  textWidth = fontRenderer ? fontRenderer->MeasureText(m_text, scale).x : 0.0F;
+		if (textWidth < visibleWidth) {
+			m_horizontalScroll = 0.0F; // No scroll needed
+		} else {
+			m_horizontalScroll = std::max(0.0F, std::min(m_horizontalScroll, textWidth - visibleWidth));
+		}
+	}
+
+	// ============================================================================
+	// Geometry Queries
+	// ============================================================================
+
+	bool TextInput::ContainsPoint(const Foundation::Vec2& point) const {
+		return point.x >= m_position.x && point.x <= m_position.x + m_size.x && point.y >= m_position.y &&
+			   point.y <= m_position.y + m_size.y;
+	}
+
+} // namespace UI


Missing test coverage for the TextInput component. The FocusManager has comprehensive test coverage (focus_manager.test.cpp), but there are no tests for TextInput which is a critical component with complex logic including UTF-8 handling, selection, clipboard operations, and mouse interaction. Given that there are tests for FocusManager in the same directory structure, TextInput should also have test coverage.

Copilot · 2025-11-26T02:46:40Z

+#include "utils/utf8.h"
+#include <cassert>
+
+namespace foundation {
+namespace UTF8 {
+
+size_t CharacterSize(unsigned char firstByte) {
+	// UTF-8 encoding:
+	// 0xxxxxxx = 1 byte (ASCII)
+	// 110xxxxx = 2 bytes
+	// 1110xxxx = 3 bytes
+	// 11110xxx = 4 bytes
+	// 10xxxxxx = continuation byte (invalid as first byte)
+
+	if ((firstByte & 0x80) == 0x00) {
+		return 1; // 0xxxxxxx
+	}
+	if ((firstByte & 0xE0) == 0xC0) {
+		return 2; // 110xxxxx
+	}
+	if ((firstByte & 0xF0) == 0xE0) {
+		return 3; // 1110xxxx
+	}
+	if ((firstByte & 0xF8) == 0xF0) {
+		return 4; // 11110xxx
+	}
+
+	// Invalid UTF-8 byte - treat as 1 byte
+	return 1;
+}
+
+size_t PreviousCharacterSize(const std::string& str, size_t offset) {
+	assert(offset > 0 && "PreviousCharacterSize: offset must be > 0");
+	assert(offset <= str.size() && "PreviousCharacterSize: offset out of bounds");
+
+	// Walk backwards from offset-1 until we find a non-continuation byte
+	size_t pos = offset - 1;
+
+	// Skip continuation bytes (10xxxxxx)
+	size_t count = 1;
+	while (pos > 0 && IsContinuationByte(static_cast<unsigned char>(str[pos]))) {
+		pos--;
+		count++;
+		if (count > 4) {
+			// Invalid UTF-8 sequence (too many continuation bytes)
+			return 1;
+		}
+	}
+
+	// Verify the character size matches what the first byte indicates
+	size_t expectedSize = CharacterSize(static_cast<unsigned char>(str[pos]));
+	if (expectedSize != count) {
+		// Invalid UTF-8 sequence - return 1 byte as fallback
+		return 1;
+	}
+
+	return count;
+}
+
+std::string Encode(char32_t codepoint) {
+	std::string result;
+
+	if (codepoint <= 0x7F) {
+		// 1 byte: 0xxxxxxx
+		result.push_back(static_cast<char>(codepoint));
+	} else if (codepoint <= 0x7FF) {
+		// 2 bytes: 110xxxxx 10xxxxxx
+		result.push_back(static_cast<char>(0xC0 | ((codepoint >> 6) & 0x1F)));
+		result.push_back(static_cast<char>(0x80 | (codepoint & 0x3F)));
+	} else if (codepoint <= 0xFFFF) {
+		// 3 bytes: 1110xxxx 10xxxxxx 10xxxxxx
+		result.push_back(static_cast<char>(0xE0 | ((codepoint >> 12) & 0x0F)));
+		result.push_back(static_cast<char>(0x80 | ((codepoint >> 6) & 0x3F)));
+		result.push_back(static_cast<char>(0x80 | (codepoint & 0x3F)));
+	} else if (codepoint <= 0x10FFFF) {
+		// 4 bytes: 11110xxx 10xxxxxx 10xxxxxx 10xxxxxx
+		result.push_back(static_cast<char>(0xF0 | ((codepoint >> 18) & 0x07)));
+		result.push_back(static_cast<char>(0x80 | ((codepoint >> 12) & 0x3F)));
+		result.push_back(static_cast<char>(0x80 | ((codepoint >> 6) & 0x3F)));
+		result.push_back(static_cast<char>(0x80 | (codepoint & 0x3F)));
+	} else {
+		// Invalid codepoint - return replacement character U+FFFD
+		result = "\xEF\xBF\xBD";
+	}
+
+	return result;
+}
+
+std::vector<char32_t> Decode(const std::string& str) {
+	std::vector<char32_t> result;
+	result.reserve(str.size()); // Worst case: all ASCII
+
+	size_t i = 0;
+	while (i < str.size()) {
+		unsigned char firstByte = static_cast<unsigned char>(str[i]);
+		size_t		  charSize = CharacterSize(firstByte);
+
+		// Check if we have enough bytes
+		if (i + charSize > str.size()) {
+			// Incomplete UTF-8 sequence - add replacement character
+			result.push_back(0xFFFD);
+			break;
+		}
+
+		char32_t codepoint = 0;
+
+		if (charSize == 1) {
+			// ASCII: 0xxxxxxx
+			codepoint = firstByte;
+		} else if (charSize == 2) {
+			// 2 bytes: 110xxxxx 10xxxxxx
+			codepoint = ((firstByte & 0x1F) << 6) | (static_cast<unsigned char>(str[i + 1]) & 0x3F);
+		} else if (charSize == 3) {
+			// 3 bytes: 1110xxxx 10xxxxxx 10xxxxxx
+			codepoint = ((firstByte & 0x0F) << 12) | ((static_cast<unsigned char>(str[i + 1]) & 0x3F) << 6) |
+						(static_cast<unsigned char>(str[i + 2]) & 0x3F);
+		} else if (charSize == 4) {
+			// 4 bytes: 11110xxx 10xxxxxx 10xxxxxx 10xxxxxx
+			codepoint = ((firstByte & 0x07) << 18) | ((static_cast<unsigned char>(str[i + 1]) & 0x3F) << 12) |
+						((static_cast<unsigned char>(str[i + 2]) & 0x3F) << 6) |
+						(static_cast<unsigned char>(str[i + 3]) & 0x3F);
+		}
+
+		result.push_back(codepoint);
+		i += charSize;
+	}
+
+	return result;
+}
+
+size_t CharacterCount(const std::string& str) {
+	size_t count = 0;
+	size_t i = 0;
+
+	while (i < str.size()) {
+		size_t charSize = CharacterSize(static_cast<unsigned char>(str[i]));
+		count++;
+		i += charSize;
+	}
+
+	return count;
+}
+
+bool IsContinuationByte(unsigned char byte) {
+	// Continuation bytes: 10xxxxxx
+	return (byte & 0xC0) == 0x80;
+}
+
+size_t NextCharacterBoundary(const std::string& str, size_t offset) {
+	if (offset >= str.size()) {
+		return str.size();
+	}
+
+	size_t charSize = CharacterSize(static_cast<unsigned char>(str[offset]));
+	return std::min(offset + charSize, str.size());
+}
+
+size_t PreviousCharacterBoundary(const std::string& str, size_t offset) {
+	if (offset == 0) {
+		return 0;
+	}
+
+	size_t charSize = PreviousCharacterSize(str, offset);
+	return offset - charSize;
+}
+
+} // namespace UTF8
+} // namespace foundation


Missing test coverage for the UTF-8 utility functions. These functions handle complex UTF-8 encoding/decoding logic with edge cases (invalid sequences, multi-byte characters, boundary conditions). Given that the repository has comprehensive test coverage for FocusManager, the UTF-8 utilities should also have tests to ensure correctness.

Move constructors and move assignment operators now preserve the original tabIndex value when re-registering with FocusManager, preventing tab order changes during std::vector reallocations or other move operations. Changes: - Added m_tabIndex member to both TextInput and Button - Initialize m_tabIndex in constructors from args.tabIndex - Include m_tabIndex in move constructor initializer lists - Assign m_tabIndex in move assignment operators - Use m_tabIndex instead of -1 when calling RegisterFocusable() Addresses PR #30 feedback from copilot-pull-request-reviewer.

kcjonson · 2025-11-26T16:41:00Z

PR Feedback Addressed

I've reviewed all 6 comments from @copilot-pull-request-reviewer and pushed a fix for the valid issues.

✅ Fixed: tabIndex Preservation (Comments #1 and #2)

Issue: Move constructors and move assignment operators for both TextInput and Button were using -1 (auto-assign) when re-registering with FocusManager, causing tab order changes during move operations.

Fix: Added m_tabIndex member to both components to preserve the original tabIndex value across moves.

Commit: 0a11764

❌ False Positive: FocusManager Get() Method (Comment #3)

Claim: "there's no Get() method or static instance mechanism defined"
Reality: FocusManager is not a singleton. The Application class creates and owns the instance (m_focusManager), and provides access via Application::GetFocusManager(). This is the correct pattern - Application owns the FocusManager instance, not a static singleton.

Evidence: libs/engine/application/application.h line 44 defines the method, and application.cpp line 227 implements it.

❌ False Positive: Missing #include (Comment #4)

Claim: "Missing #include <cassert>"
Reality: Already included at line 3 of focus_manager.cpp

Evidence: libs/ui/focus/focus_manager.cpp line 3 shows #include <cassert>

ℹ️ Comment #5 and #6

Comments #5 and #6 appear to have been truncated in the API response. If there are additional concerns, please let me know.

Copilot · 2025-11-26T16:41:12Z

@kcjonson I've opened a new pull request, #31, to work on those changes. Once the pull request is ready, I'll request review from you.

kcjonson · 2025-11-26T16:36:18Z

+					.text = "",
+					.placeholder = "Basic text input (Tab index 0)",
+					.style = {},
+					.focusManager = &UI::FocusManager::Get(),


I don't like how the focus manager is passed in as a property, isn't it a singleton? Shouldn't there be a global way to get the focus manager without supplying it specifically?

kcjonson · 2025-11-26T16:37:05Z

+					.size = {400.0F, 40.0F},
+					.text = "",
+					.placeholder = "Basic text input (Tab index 0)",
+					.style = {},


Can empty properties be emitted completely? Isn't it ok to remove them as long as the order is correct? is style not nullable?

kcjonson · 2025-11-26T16:39:00Z

+				std::make_unique<UI::TextInput>(UI::TextInput::Args{
+					.position = {50.0F, yPos},
+					.size = {400.0F, 40.0F},
+					.text = "Initial text with Unicode: 你好 😀",


Unicode text is not rendering properly in the sandbox demo. just ????? perhaps the characters are not in our font set? I assume this will require regenerating the SDF fonts atlas.

We don't need to (or want really) to allow emojii, this should be explicitly blocked

kcjonson · 2025-11-26T16:40:20Z

+					.zIndex = 5.0F,
+					.enabled = true,
+					.onChange = [this](const std::string& text) { m_output3 = "Input 3 (numbers): " + text; },
+					.charValidator = [](char32_t c) { return (c >= '0' && c <= '9') || c == '.' || c == '-'; }


Its an interaction anti-pattern to hard block people from being able to type in a field, since it appears that the field is broken. I thought this while testing. We should remove the charValidator functionality from text input completely.

kcjonson · 2025-11-26T16:43:40Z

+
+		// Delete selection first if active
+		if (m_selection.has_value() && !m_selection->IsEmpty()) {
+			LOG_INFO(UI, "InsertChar: deleting selection before insert");


Remove logging

kcjonson · 2025-11-26T16:43:47Z

+			LOG_INFO(UI, "InsertChar: deleting selection before insert");
+			DeleteSelection();
+		} else {
+			LOG_INFO(UI, "InsertChar: no selection, inserting at cursor");


remove logging

kcjonson · 2025-11-26T16:47:35Z

+		}
+
+		// Get batch renderer
+		ui::TextBatchRenderer* batchRenderer = Renderer::Primitives::GetTextBatchRenderer();


In many of the render methods we're fetching these utilities and doing some calculations, perhaps these should be moved to a parent render function and supplied as parameters to the specific render functions, or maybe even set as a private member on the class for general reuse.

kcjonson · 2025-11-26T16:50:18Z

+		}
+
+		// Set clipboard using GLFW
+		GLFWwindow* window = glfwGetCurrentContext();


I don't love accessing GLFW directly here, don't we have an abstraction somewhere for window information? If not we should. Maybe a specific clipboard helper?

kcjonson · 2025-11-26T16:52:27Z

+
+		// Clamp to ensure text fills from left when possible
+		// Convert fontSize to scale (16px base size = 1.0 scale)
+		constexpr float BASE_FONT_SIZE = 16.0F;


This constant should certainly not be hard coded here, I would expect the font renderer to be able to supply this. Isn't this determined at the time we create the font atlas?

Copilot

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-26T17:08:50Z

+			m_focused = other.m_focused;
+			m_mouseDown = other.m_mouseDown;
+			m_focusManager = other.m_focusManager;
+		m_tabIndex = other.m_tabIndex;


Duplicate assignment of m_tabIndex. Line 101 assigns m_tabIndex = other.m_tabIndex; which is redundant since it's already assigned on line 101 (appears to be a copy-paste error).

Suggested change

m_tabIndex = other.m_tabIndex;

m_tabIndex = other.m_tabIndex;

Copilot · 2025-11-26T17:08:50Z

+			m_tabIndex = other.m_tabIndex;
+


Duplicate assignment of m_tabIndex. Line 115 assigns m_tabIndex = other.m_tabIndex; which is already assigned on line 114.

Suggested change

m_tabIndex = other.m_tabIndex;

Copilot · 2025-11-26T17:08:50Z

+		if (m_selection.has_value()) {
+		}


Empty conditional block. Lines 444-445 contain an if statement with an empty body, which serves no purpose and should be removed.

Suggested change

if (m_selection.has_value()) {

}

Copilot

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-26T17:46:50Z

+					.text = "",
+					.placeholder = "Basic text input (Tab index 0)",
+					.style = {},
+					.focusManager = &UI::FocusManager::Get(),


FocusManager does not appear to have a static Get() method based on the header file (focus_manager.h shows no singleton pattern). This code will likely fail to compile. The FocusManager should be accessed via Application::GetFocusManager() as shown in the application integration.

Copilot · 2025-11-26T17:46:50Z


-		std::unique_ptr<InputManager> m_inputManager; // Input management system
+		std::unique_ptr<InputManager>		m_inputManager;	  // Input management system
+	std::unique_ptr<ClipboardManager>	m_clipboardManager; // Clipboard management system


[nitpick] Inconsistent indentation/formatting: this line uses tabs while line 131 uses spaces. Maintain consistent whitespace formatting across member variable declarations.

Suggested change

std::unique_ptr<ClipboardManager> m_clipboardManager; // Clipboard management system

std::unique_ptr<ClipboardManager> m_clipboardManager; // Clipboard management system

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-26T19:57:15Z

+		// Unregister other from its old address, register this at new address
+		if (m_focusManager != nullptr) {
+			m_focusManager->UnregisterFocusable(&other);
+			// We need to register with the same tabIndex, but FocusManager doesn't store it separately
+			// For now, just register with auto-assign
+			m_focusManager->RegisterFocusable(this, -1);
+		}
+		other.m_focusManager = nullptr; // Prevent double-unregister


The move constructor and move assignment operator (lines 80-86, 114-116) do not preserve the original tabIndex when re-registering with the FocusManager, using -1 (auto-assign) instead. This could change the tab order after a move operation. Consider storing the tabIndex or documenting this limitation.

Copilot · 2025-11-26T19:57:16Z

+			codepoint = ((firstByte & 0x1F) << 6) | (static_cast<unsigned char>(str[i + 1]) & 0x3F);
+		} else if (charSize == 3) {
+			// 3 bytes: 1110xxxx 10xxxxxx 10xxxxxx
+			codepoint = ((firstByte & 0x0F) << 12) | ((static_cast<unsigned char>(str[i + 1]) & 0x3F) << 6) |
+						(static_cast<unsigned char>(str[i + 2]) & 0x3F);
+		} else if (charSize == 4) {
+			// 4 bytes: 11110xxx 10xxxxxx 10xxxxxx 10xxxxxx
+			codepoint = ((firstByte & 0x07) << 18) | ((static_cast<unsigned char>(str[i + 1]) & 0x3F) << 12) |
+						((static_cast<unsigned char>(str[i + 2]) & 0x3F) << 6) |
+						(static_cast<unsigned char>(str[i + 3]) & 0x3F);


The Decode function in UTF-8 utilities does not validate that continuation bytes actually start with 10xxxxxx. This could result in incorrect decoding of malformed UTF-8 sequences. Consider adding validation using the IsContinuationByte() function (line 144) for bytes at positions i+1, i+2, and i+3 before using them.

Suggested change

codepoint = ((firstByte & 0x1F) << 6) | (static_cast<unsigned char>(str[i + 1]) & 0x3F);

} else if (charSize == 3) {

// 3 bytes: 1110xxxx 10xxxxxx 10xxxxxx

codepoint = ((firstByte & 0x0F) << 12) | ((static_cast<unsigned char>(str[i + 1]) & 0x3F) << 6) |

(static_cast<unsigned char>(str[i + 2]) & 0x3F);

} else if (charSize == 4) {

// 4 bytes: 11110xxx 10xxxxxx 10xxxxxx 10xxxxxx

codepoint = ((firstByte & 0x07) << 18) | ((static_cast<unsigned char>(str[i + 1]) & 0x3F) << 12) |

((static_cast<unsigned char>(str[i + 2]) & 0x3F) << 6) |

(static_cast<unsigned char>(str[i + 3]) & 0x3F);

if (!IsContinuationByte(static_cast<unsigned char>(str[i + 1]))) {

// Invalid continuation byte

codepoint = 0xFFFD;

charSize = 1; // Skip only the first byte

} else {

codepoint = ((firstByte & 0x1F) << 6) | (static_cast<unsigned char>(str[i + 1]) & 0x3F);

}

} else if (charSize == 3) {

// 3 bytes: 1110xxxx 10xxxxxx 10xxxxxx

if (!IsContinuationByte(static_cast<unsigned char>(str[i + 1])) ||

!IsContinuationByte(static_cast<unsigned char>(str[i + 2]))) {

// Invalid continuation byte(s)

codepoint = 0xFFFD;

charSize = 1;

} else {

codepoint = ((firstByte & 0x0F) << 12) | ((static_cast<unsigned char>(str[i + 1]) & 0x3F) << 6) |

(static_cast<unsigned char>(str[i + 2]) & 0x3F);

}

} else if (charSize == 4) {

// 4 bytes: 11110xxx 10xxxxxx 10xxxxxx 10xxxxxx

if (!IsContinuationByte(static_cast<unsigned char>(str[i + 1])) ||

!IsContinuationByte(static_cast<unsigned char>(str[i + 2])) ||

!IsContinuationByte(static_cast<unsigned char>(str[i + 3]))) {

// Invalid continuation byte(s)

codepoint = 0xFFFD;

charSize = 1;

} else {

codepoint = ((firstByte & 0x07) << 18) | ((static_cast<unsigned char>(str[i + 1]) & 0x3F) << 12) |

((static_cast<unsigned char>(str[i + 2]) & 0x3F) << 6) |

(static_cast<unsigned char>(str[i + 3]) & 0x3F);

}

Copilot · 2025-11-26T19:57:16Z

+		if (m_text.empty() && !m_focused) {
+			RenderPlaceholder();
+		} else {
+			RenderSelection(); // Phase 2 (currently stub)


The comment on line 212 says "Phase 2 (currently stub)" but RenderSelection() is actually implemented (lines 715-757). This comment is outdated and should be updated to reflect that selection rendering is fully implemented.

Suggested change

RenderSelection(); // Phase 2 (currently stub)

RenderSelection(); // Render selection highlight if any

Copilot · 2025-11-26T19:57:17Z

+		if (m_focusManager != nullptr) {
+			m_focusManager->UnregisterFocusable(&other);
+			m_focusManager->RegisterFocusable(this, -1);


The move constructor does not preserve the original tabIndex when re-registering with the FocusManager. It uses -1 (auto-assign) instead, which could change the tab order. The same issue exists in the move assignment operator (line 107). Consider storing the tabIndex or documenting this limitation.

…on, code cleanup - Add FocusManager static instance pattern (Get/SetInstance) like InputManager - Add m_tabIndex preservation for Button and TextInput move operations - Create ClipboardManager abstraction to replace direct GLFW clipboard calls - Remove charValidator anti-pattern from TextInput - Consolidate BASE_FONT_SIZE constant to file-level kBaseFontSize - Remove debug LOG_INFO calls from TextInput::InsertChar - Fix startup ordering: Application must be created before scene loading - Add text_input_scene.cpp to CMakeLists.txt (was missing from build)

On macOS, Cmd is GLFW_MOD_SUPER not GLFW_MOD_CONTROL. Added platform check to treat Cmd as ctrl for standard shortcuts.

Copilot

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-26T20:28:35Z

+	FocusManager* m_focusManager{nullptr};
+	int			  m_tabIndex{-1}; // Preserved for move operations


[nitpick] The m_tabIndex member is declared after m_focusManager in the private section, but in the move constructor initialization list on line 72, it's accessed before checking if m_focusManager is null on line 75. If m_tabIndex depends on being valid for the RegisterFocusable call on line 77, this ordering should be consistent. Consider reordering the member declarations to make the dependency clearer.

Suggested change

FocusManager* m_focusManager{nullptr};

int m_tabIndex{-1}; // Preserved for move operations

int m_tabIndex{-1}; // Preserved for move operations

FocusManager* m_focusManager{nullptr};

Copilot · 2025-11-26T20:28:35Z

+		LOG_INFO(UI, "SetSelection(%zu, %zu)", start, end);
+	}
+
+	void TextInput::ClearSelection() {
+		if (m_selection.has_value()) {
+			LOG_INFO(UI, "ClearSelection (was %zu-%zu)", m_selection->start, m_selection->end);
+		}
+		m_selection.reset();
+	}
+
+	std::string TextInput::GetSelectedText() const {
+		if (!m_selection.has_value() || m_selection->IsEmpty()) {
+			return "";
+		}
+
+		size_t start = m_selection->GetMin();
+		size_t end = m_selection->GetMax();
+		return m_text.substr(start, end - start);
+	}
+
+	void TextInput::DeleteSelection() {
+		if (!m_selection.has_value() || m_selection->IsEmpty()) {
+			LOG_INFO(UI, "DeleteSelection called but no selection");
+			return;
+		}
+
+		size_t start = m_selection->GetMin();
+		size_t end = m_selection->GetMax();
+		LOG_INFO(UI, "DeleteSelection deleting range %zu-%zu", start, end);


The logging calls on lines 448, 453, 470, and 476 use LOG_INFO for selection operations. These are per-operation logs that could fire frequently during text editing (every time selection changes). Consider whether these should remain at INFO level or be reduced to DEBUG level for production use, or removed entirely as mentioned in the PR description about "removing excessive per-frame logging for better performance".

Copilot · 2025-11-26T20:28:35Z

+		std::vector<char32_t> codepoints = foundation::UTF8::Decode(pasteText);
+
+		for (char32_t codepoint : codepoints) {
+			// Filter newlines and control characters


[nitpick] In the Paste() function, the code filters out newlines and carriage returns (lines 577-578), but the comment on line 576 says "Filter newlines and control characters". However, the code also checks codepoint != '\t' which would exclude tabs. The condition (codepoint < 32 && codepoint != '\t') means tabs ARE allowed (since '\t' == 9), but this creates inconsistent behavior: newlines/returns are explicitly filtered despite being < 32, while other control characters < 32 (except tab) are filtered. This logic is confusing and could lead to unexpected behavior. Consider clarifying the intent and making the filtering logic more explicit.

Suggested change

// Filter newlines and control characters

// Filter out newlines, carriage returns, and all control characters except tab.

- Remove all LOG_INFO and LOG_DEBUG statements - Remove unused log.h include - Update outdated 'Phase 2 stub' comment

Block emoji ranges only (U+1F000-U+1FAFF, U+2600-U+27BF, variation selectors). Unicode characters like CJK are allowed for internationalization support. Filters both typing and paste operations.

Copilot

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-26T20:34:52Z

+			if (m_focused && m_mouseDown && input.IsMouseButtonDown(engine::MouseButton::Left)) {
+				Foundation::Vec2 mousePos = input.GetMousePosition();
+				float			 localX = mousePos.x - m_position.x - m_style.paddingLeft + m_horizontalScroll;
+				size_t			 dragPosition = GetCursorPositionFromMouse(localX);
+
+				// Create/update selection from original cursor position to drag position
+				if (dragPosition != m_cursorPosition) {
+					// Selection anchor is where mouse was first pressed (already set as m_cursorPosition)
+					// Selection head is current drag position
+					if (!m_selection.has_value()) {
+						// Start new selection
+						SetSelection(m_cursorPosition, dragPosition);
+					} else {
+						// Update existing selection
+						m_selection->end = dragPosition;
+					}
+					m_cursorPosition = dragPosition; // Move cursor to drag position
+					UpdateHorizontalScroll();
+				}
+			}


The logic for drag selection has a potential issue. Lines 156-175 check m_focused && m_mouseDown for drag selection, but m_mouseDown is only set to true when clicking inside the text input (line 151). However, if the user clicks on another text input field, the first field loses focus (OnFocusLost is called) but m_mouseDown remains true, which could cause ghost selections.

The fix on line 156 (checking m_focused) prevents the ghost selection behavior described in the PR, but there's still a logical inconsistency: m_mouseDown should be cleared when focus is lost to maintain clean state.

Consider clearing m_mouseDown in OnFocusLost() to ensure complete state cleanup.

When a TextInput loses focus (e.g., clicking on another field), the m_mouseDown flag was not being cleared. This could cause ghost selection behavior if the user later interacted with the unfocused field while it still thought the mouse was down.

Components (TextInput, Button) now always auto-register with the global FocusManager singleton via FocusManager::Get(). This simplifies component creation by removing the need to explicitly pass a focusManager pointer. Changes: - Remove focusManager from TextInput::Args and Button::Args - Remove m_focusManager member from both components - Update constructors/destructors/move operations to use Get() - Update text_input_scene.cpp to use simplified Args

Copilot

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-26T20:51:27Z

+			}
+
+			// Mouse drag selection
+			if (m_focused && m_mouseDown && input.IsMouseButtonDown(engine::MouseButton::Left)) {


Line 141 has redundant check: input.IsMouseButtonDown(engine::MouseButton::Left) is already checked at line 116, so this condition will always be true inside this block. Consider simplifying to if (m_focused && m_mouseDown).

Suggested change

if (m_focused && m_mouseDown && input.IsMouseButtonDown(engine::MouseButton::Left)) {

if (m_focused && m_mouseDown) {

Copilot · 2025-11-26T20:51:27Z

+	// Create application BEFORE scene loading (FocusManager, InputManager, etc. need to exist)
+	LOG_INFO(UI, "Creating application");
+	engine::Application app(window);
+
 	// Initialize scene system
 	LOG_INFO(Engine, "Initializing scene system");


[nitpick] The comment at line 382 explains why Application is created before scene loading, but this ordering constraint is subtle and could cause issues if violated in the future. Consider adding an assertion or runtime check in scene initialization to verify that FocusManager is available, rather than relying solely on comments.

kcjonson added 4 commits November 25, 2025 12:20

Copilot AI review requested due to automatic review settings November 26, 2025 02:43

Copilot started reviewing on behalf of kcjonson November 26, 2025 02:44

View session

Copilot finished reviewing on behalf of kcjonson November 26, 2025 02:45

Copilot AI reviewed Nov 26, 2025

View reviewed changes

kcjonson commented Nov 26, 2025

View reviewed changes

Comment thread libs/ui/components/text_input/text_input.h Resolved

Copilot AI mentioned this pull request Nov 26, 2025

Reply to naming convention question on TextInput.GetText() #31

Closed

kcjonson commented Nov 26, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings November 26, 2025 17:06

Copilot started reviewing on behalf of kcjonson November 26, 2025 17:07

View session

Copilot finished reviewing on behalf of kcjonson November 26, 2025 17:08

Copilot AI reviewed Nov 26, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings November 26, 2025 17:44

Copilot started reviewing on behalf of kcjonson November 26, 2025 17:45

View session

Copilot finished reviewing on behalf of kcjonson November 26, 2025 17:45

Copilot AI reviewed Nov 26, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings November 26, 2025 19:53

kcjonson force-pushed the fix/text-input-key-repeat-and-selection-bugs branch from 28393bd to c2e1daa Compare November 26, 2025 19:53

Copilot started reviewing on behalf of kcjonson November 26, 2025 19:53

View session

Copilot finished reviewing on behalf of kcjonson November 26, 2025 19:55

Copilot AI reviewed Nov 26, 2025

View reviewed changes

kcjonson added 2 commits November 26, 2025 12:21

fix: support Cmd key for copy/paste shortcuts on macOS

7777d8d

On macOS, Cmd is GLFW_MOD_SUPER not GLFW_MOD_CONTROL. Added platform check to treat Cmd as ctrl for standard shortcuts.

Copilot AI review requested due to automatic review settings November 26, 2025 20:26

Copilot started reviewing on behalf of kcjonson November 26, 2025 20:26

View session

Copilot finished reviewing on behalf of kcjonson November 26, 2025 20:27

Copilot AI reviewed Nov 26, 2025

View reviewed changes

chore: remove remaining debug logging from TextInput

4b504b1

- Remove all LOG_INFO and LOG_DEBUG statements - Remove unused log.h include - Update outdated 'Phase 2 stub' comment

Copilot AI review requested due to automatic review settings November 26, 2025 20:32

Copilot started reviewing on behalf of kcjonson November 26, 2025 20:33

View session

fix: filter emoji from TextInput (allow other Unicode)

e71461c

Block emoji ranges only (U+1F000-U+1FAFF, U+2600-U+27BF, variation selectors). Unicode characters like CJK are allowed for internationalization support. Filters both typing and paste operations.

kcjonson force-pushed the fix/text-input-key-repeat-and-selection-bugs branch from 4571ddf to e71461c Compare November 26, 2025 20:34

Copilot finished reviewing on behalf of kcjonson November 26, 2025 20:34

Copilot AI reviewed Nov 26, 2025

View reviewed changes

kcjonson added 2 commits November 26, 2025 12:43

Copilot AI review requested due to automatic review settings November 26, 2025 20:49

Copilot started reviewing on behalf of kcjonson November 26, 2025 20:50

View session

Copilot finished reviewing on behalf of kcjonson November 26, 2025 20:50

Copilot AI reviewed Nov 26, 2025

View reviewed changes

kcjonson merged commit a0e75fb into main Nov 26, 2025
8 of 9 checks passed

	m_tabIndex = other.m_tabIndex;
	m_tabIndex = other.m_tabIndex;

	std::unique_ptr<ClipboardManager> m_clipboardManager; // Clipboard management system
	std::unique_ptr<ClipboardManager> m_clipboardManager; // Clipboard management system

	RenderSelection(); // Phase 2 (currently stub)
	RenderSelection(); // Render selection highlight if any

		FocusManager* m_focusManager{nullptr};
		int m_tabIndex{-1}; // Preserved for move operations

	// Filter newlines and control characters
	// Filter out newlines, carriage returns, and all control characters except tab.

	if (m_focused && m_mouseDown && input.IsMouseButtonDown(engine::MouseButton::Left)) {
	if (m_focused && m_mouseDown) {

Conversation

kcjonson commented Nov 26, 2025

Summary

Bugs Fixed

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kcjonson commented Nov 26, 2025

PR Feedback Addressed

✅ Fixed: tabIndex Preservation (Comments #1 and #2)

❌ False Positive: FocusManager Get() Method (Comment #3)

❌ False Positive: Missing #include (Comment #4)

ℹ️ Comment #5 and #6

Uh oh!

Copilot AI commented Nov 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment