From 6709f52e0f21211f6b35ad15716adc4877987bd5 Mon Sep 17 00:00:00 2001 From: Greg Pomerantz Date: Mon, 1 Jun 2026 10:57:34 -0400 Subject: [PATCH] Fix browser page scrolling by initializing EntryHeight and correcting scroll handler type assertion --- doc/browser_implementation_plan.md | 24 +- internal/browser/browser.go | 20 +- internal/browser/browser_test.go | 47 ++-- internal/browser/handlers.go | 29 ++- internal/browser/index.go | 21 +- internal/browser/layout.go | 98 +++++++- internal/browser/layout_test.go | 30 +-- internal/browser/pixel_scroll_test.go | 258 +++++++++++++++++++++ internal/browser/scroll_test.go | 114 +++++----- internal/browser/search.go | 6 +- internal/browser/search_test.go | 19 +- internal/browser/types.go | 26 ++- internal/browser/types_test.go | 15 +- internal/editor/state.go | 13 +- internal/test/e2e/browser_files_test.go | 4 +- internal/test/e2e/search_test.go | 284 ++++++++++++++++++++++++ internal/ui/render.go | 2 + 17 files changed, 855 insertions(+), 155 deletions(-) create mode 100644 internal/browser/pixel_scroll_test.go create mode 100644 internal/test/e2e/search_test.go diff --git a/doc/browser_implementation_plan.md b/doc/browser_implementation_plan.md index e65c7b0..4488d18 100644 --- a/doc/browser_implementation_plan.md +++ b/doc/browser_implementation_plan.md @@ -85,7 +85,8 @@ const ( type BrowserState struct { // Navigation CurrentPath string // Currently browsed directory (relative to root) - ScrollIndex int // Index of first visible entry (not pixel offset) + ScrollOffset float64 // Vertical scroll offset in pixels + EntryHeight float64 // Height of a single entry in pixels VisibleCount int // Number of entries currently visible // Lazy loading @@ -455,25 +456,20 @@ func HandleBrowserScroll(data any) { defer state.Unlock() scrollData := data.(ScrollEvent) - delta := int(scrollData.Delta / state.Browser.EntryHeight) - - oldScrollIndex := state.Browser.ScrollIndex - state.Browser.ScrollIndex += delta + state.Browser.ScrollOffset += scrollData.Delta // Clamp - maxScroll := state.Browser.TotalEntries - state.Browser.VisibleCount - if state.Browser.ScrollIndex < 0 { - state.Browser.ScrollIndex = 0 + maxScroll := float64(state.Browser.TotalEntries)*state.Browser.EntryHeight - viewHeight + if state.Browser.ScrollOffset < 0 { + state.Browser.ScrollOffset = 0 } - if state.Browser.ScrollIndex > maxScroll { - state.Browser.ScrollIndex = maxScroll + if state.Browser.ScrollOffset > maxScroll { + state.Browser.ScrollOffset = maxScroll } // Trigger prefetch for newly visible pages - if state.Browser.ScrollIndex != oldScrollIndex { - dispatchPrefetchPages(state.Browser) - state.Browser.EvictPages() - } + dispatchPrefetchPages(state.Browser) + state.Browser.EvictPages() } ``` diff --git a/internal/browser/browser.go b/internal/browser/browser.go index 469bce0..ae32a84 100644 --- a/internal/browser/browser.go +++ b/internal/browser/browser.go @@ -62,11 +62,11 @@ func loadPageFromIndex(s *BrowserState, pageIndex int) *Page { // computeVisiblePageRange returns the min and max page indices that should // be loaded based on the current scroll position and visible count. func computeVisiblePageRange(s *BrowserState) (minPage, maxPage int) { - minPage = s.ScrollIndex/PageSize - PrefetchDist + minPage = s.GetScrollIndex()/PageSize - PrefetchDist if minPage < 0 { minPage = 0 } - maxPage = (s.ScrollIndex + s.VisibleCount)/PageSize + PrefetchDist + maxPage = (s.GetScrollIndex() + s.VisibleCount)/PageSize + PrefetchDist return minPage, maxPage } @@ -121,7 +121,7 @@ func needsPrefetch(s *BrowserState, pageIndex int) (bool, int) { // navigateToDirectory resets browser state and sets the new current path. func navigateToDirectory(s *BrowserState, path string) { s.CurrentPath = path - s.ScrollIndex = 0 + s.ScrollOffset = 0 s.SelectedIndex = -1 s.Pages = make(map[int]*Page) s.SearchResults = nil @@ -141,19 +141,7 @@ func computeTotalPages(s *BrowserState) int { return pages } -// clampScrollIndex ensures the scroll index stays within valid bounds. -func clampScrollIndex(s *BrowserState) { - if s.ScrollIndex < 0 { - s.ScrollIndex = 0 - } - maxScroll := s.TotalEntries - s.VisibleCount - if maxScroll < 0 { - maxScroll = 0 - } - if s.ScrollIndex > maxScroll { - s.ScrollIndex = maxScroll - } -} + // filterEntriesByQuery returns indices of entries matching the query (case-insensitive). // Returns nil for empty query. diff --git a/internal/browser/browser_test.go b/internal/browser/browser_test.go index 2546dcf..b470f2b 100644 --- a/internal/browser/browser_test.go +++ b/internal/browser/browser_test.go @@ -126,12 +126,13 @@ func TestLoadPage_OutOfBoundsFromIndex(t *testing.T) { func TestComputeVisiblePageRange(t *testing.T) { s := NewBrowserState() - s.ScrollIndex = 250 + s.EntryHeight = 48.0 + s.ScrollOffset = 250 * 48 // 250 entries worth of pixels s.VisibleCount = 20 minPage, maxPage := computeVisiblePageRange(s) - // ScrollIndex=250 → page 2 + // ScrollOffset=250*48 → GetScrollIndex()=250 → page 2 // VisibleCount=20 → entries 250-269 → pages 2-2 (partial) // With prefetch: min=0, max=4 expectedMin := 0 // (250/100) - 2 = 0 @@ -149,7 +150,8 @@ func TestComputeVisiblePageRange(t *testing.T) { func TestComputeVisiblePageRange_AtStart(t *testing.T) { s := NewBrowserState() - s.ScrollIndex = 0 + s.EntryHeight = 48.0 + s.ScrollOffset = 0 s.VisibleCount = 10 minPage, maxPage := computeVisiblePageRange(s) @@ -254,7 +256,8 @@ func TestGetEntryByIndex_OutOfBounds(t *testing.T) { func TestNeedsPrefetch(t *testing.T) { s := NewBrowserState() - s.ScrollIndex = 500 + s.EntryHeight = 48.0 + s.ScrollOffset = 500 * 48 // 500 entries worth of pixels s.VisibleCount = 20 s.TotalEntries = 2000 @@ -281,8 +284,9 @@ func TestNeedsPrefetch(t *testing.T) { func TestNavigateToDirectory(t *testing.T) { s := NewBrowserState() + s.EntryHeight = 48.0 s.CurrentPath = "/old/path" - s.ScrollIndex = 100 + s.ScrollOffset = 100 * 48 // 100 entries worth of pixels s.SelectedIndex = 5 s.Pages[0] = NewPage(0, nil) @@ -291,8 +295,8 @@ func TestNavigateToDirectory(t *testing.T) { if s.CurrentPath != "/new/path" { t.Errorf("expected CurrentPath=/new/path, got %s", s.CurrentPath) } - if s.ScrollIndex != 0 { - t.Errorf("expected ScrollIndex=0, got %d", s.ScrollIndex) + if s.ScrollOffset != 0 { + t.Errorf("expected ScrollOffset=0, got %.1f", s.ScrollOffset) } if len(s.Pages) != 0 { t.Errorf("expected Pages to be empty, got %d", len(s.Pages)) @@ -327,30 +331,33 @@ func TestComputeTotalPages(t *testing.T) { // --- Test: Clamp Scroll Index --- -func TestClampScrollIndex(t *testing.T) { +func TestClampScrollOffset(t *testing.T) { s := NewBrowserState() s.TotalEntries = 500 s.VisibleCount = 20 + s.EntryHeight = 48.0 // Normal case - s.ScrollIndex = 100 - clampScrollIndex(s) - if s.ScrollIndex != 100 { - t.Errorf("expected ScrollIndex=100, got %d", s.ScrollIndex) + s.ScrollOffset = 100 * 48 + clampScrollOffset(s) + expected := float64(100 * 48) + if s.ScrollOffset != expected { + t.Errorf("expected ScrollOffset=%.1f, got %.1f", expected, s.ScrollOffset) } // Below minimum - s.ScrollIndex = -10 - clampScrollIndex(s) - if s.ScrollIndex != 0 { - t.Errorf("expected ScrollIndex=0, got %d", s.ScrollIndex) + s.ScrollOffset = -10 * 48 + clampScrollOffset(s) + if s.ScrollOffset != 0 { + t.Errorf("expected ScrollOffset=0, got %.1f", s.ScrollOffset) } // Above maximum - s.ScrollIndex = 1000 - clampScrollIndex(s) - if s.ScrollIndex != 480 { - t.Errorf("expected ScrollIndex=480, got %d", s.ScrollIndex) + s.ScrollOffset = 1000 * 48 + clampScrollOffset(s) + maxScroll := float64(500-20) * 48 + if s.ScrollOffset != maxScroll { + t.Errorf("expected ScrollOffset=%.1f, got %.1f", maxScroll, s.ScrollOffset) } } diff --git a/internal/browser/handlers.go b/internal/browser/handlers.go index 4887ee2..188b81f 100644 --- a/internal/browser/handlers.go +++ b/internal/browser/handlers.go @@ -3,11 +3,36 @@ package browser // HandleScroll updates the browser scroll index by the given delta (in entries). // Clamps to valid bounds and triggers prefetch/eviction as needed. func HandleScroll(s *BrowserState, delta int) { - s.ScrollIndex += delta + deltaPx := int(float64(delta) * s.EntryHeight) + HandlePixelScroll(s, deltaPx) +} + +// HandlePixelScroll updates the browser scroll offset by the given delta (in pixels). +// Clamps to valid bounds and triggers eviction as needed. +func HandlePixelScroll(s *BrowserState, delta int) { + s.ScrollOffset += float64(delta) // Clamp to valid bounds - clampScrollIndex(s) + clampScrollOffset(s) // Trigger eviction of distant pages s.EvictPages() } + +// clampScrollOffset ensures the scroll offset stays within valid bounds. +func clampScrollOffset(s *BrowserState) { + if s.ScrollOffset < 0 { + s.ScrollOffset = 0 + } + + // If there's no scrollable area (entries fit in viewport), clamp to 0 + if s.TotalEntries <= s.VisibleCount { + s.ScrollOffset = 0 + return + } + + maxScroll := float64(s.TotalEntries-s.VisibleCount) * s.EntryHeight + if s.ScrollOffset > maxScroll { + s.ScrollOffset = maxScroll + } +} diff --git a/internal/browser/index.go b/internal/browser/index.go index b7fbc8a..9986b75 100644 --- a/internal/browser/index.go +++ b/internal/browser/index.go @@ -135,6 +135,8 @@ func modeCount() int { // loadCachedIndex attempts to load a cached index from disk. // Returns the index and true if valid, or nil and false if not. +// Cache validation uses both mtime and entry count to handle +// filesystems where mtime doesn't update immediately. func loadCachedIndex(dirPath string) (*DirectoryIndex, bool) { cachePath := getCachePath(dirPath) @@ -154,8 +156,23 @@ func loadCachedIndex(dirPath string) (*DirectoryIndex, bool) { return nil, false } - // If mtime matches, cache is valid - if idx.Mtime.Equal(dirInfo.ModTime()) { + // Also check entry count as a secondary invalidation signal. + // Directory mtime may not update immediately on some filesystems + // (e.g., tmpfs), but the entry count will always reflect changes. + actualEntries, err := os.ReadDir(dirPath) + if err != nil { + return nil, false + } + // Count actual entries (excluding .pad and .git) + actualCount := 0 + for _, e := range actualEntries { + if e.Name() != ".pad" && e.Name() != ".git" { + actualCount++ + } + } + + // Cache is valid only if BOTH mtime and entry count match + if idx.Mtime.Equal(dirInfo.ModTime()) && actualCount == idx.EntryCount { return &idx, true } diff --git a/internal/browser/layout.go b/internal/browser/layout.go index c584790..013e863 100644 --- a/internal/browser/layout.go +++ b/internal/browser/layout.go @@ -1,6 +1,8 @@ package browser import ( + "strings" + "pad/internal/ui" ) @@ -57,15 +59,25 @@ func BrowserLayout(screenW, screenH ui.Dp, state *BrowserState, sortHandler func // Create a scroll handler that captures the current browser state scrollHandler := func(data any) { - delta := data.(int) // entries - HandleScroll(state, delta) + var delta int + switch v := data.(type) { + case int: + delta = v + case float32: + delta = int(v) + case float64: + delta = int(v) + default: + return + } + HandlePixelScroll(state, delta) } listView := ui.NewListView( "browser_list", visibleEntries, listRegion, - ui.Dp(state.ScrollIndex)*ui.Dp(48), // scroll offset in pixels + ui.Dp(state.ScrollOffset), // scroll offset in pixels (per-pixel) state.SelectedIndex, []ui.Interaction{{Gesture: ui.Scroll, Handler: scrollHandler}}, ) @@ -94,6 +106,54 @@ func sortModeLabel(mode SortMode) string { } } +// HandleSortModeChange recomputes search results when the sort mode changes. +// This ensures SearchResults indices remain valid after pages are reloaded +// with the new sort order. +func HandleSortModeChange(s *BrowserState) { + // If there's an active search, recompute the results with the new sort order + if s.Query != "" && s.SortIndex != nil { + recomputeSearchResults(s) + } +} + +// recomputeSearchResults rebuilds SearchResults based on the current sort order. +// It checks each entry in the SortIndex against the query and updates the indices +// to reflect the new sort order. +func recomputeSearchResults(s *BrowserState) { + if s.Query == "" || s.SortIndex == nil { + return + } + + queryLower := strings.ToLower(s.Query) + var results []int + + // Build a map of entry names to their new sorted indices + // We need to find which entries match the query in the new sort order + positionMap := s.getSortedIndices() + if positionMap == nil { + return + } + + // For each raw entry, check if it matches the query + // and add its new sorted index to results + for sortedIdx, rawIdx := range positionMap { + if rawIdx >= len(s.SortIndex.Entries) { + continue + } + entry := s.SortIndex.Entries[rawIdx] + if strings.Contains(strings.ToLower(entry.Name), queryLower) { + results = append(results, sortedIdx) + } + } + + s.SearchResults = results + + // Jump to first result if any matches found + if len(results) > 0 { + s.ScrollOffset = float64(results[0]) * s.EntryHeight + } +} + // ComputeVisibleEntriesForTest is exported for testing purposes. @@ -104,12 +164,18 @@ func ComputeVisibleEntriesForTest(state *BrowserState) []ui.ListItem { // computeVisibleEntries builds the list of ui.ListItem entries that should be // rendered based on the current scroll position and visible count. +// When SearchResults is non-empty, only matching entries are shown. func computeVisibleEntries(state *BrowserState) []ui.ListItem { if state.TotalEntries == 0 { return nil } - startIndex := state.ScrollIndex + // When search is active, show only matching entries + if len(state.SearchResults) > 0 { + return computeSearchResults(state) + } + + startIndex := state.GetScrollIndex() endIndex := startIndex + state.VisibleCount if endIndex > state.TotalEntries { endIndex = state.TotalEntries @@ -133,3 +199,27 @@ func computeVisibleEntries(state *BrowserState) []ui.ListItem { return items } + +// computeSearchResults builds the list of ui.ListItem entries for search results. +// Only entries whose indices are in SearchResults are shown. +func computeSearchResults(state *BrowserState) []ui.ListItem { + var items []ui.ListItem + for _, idx := range state.SearchResults { + if idx < 0 || idx >= state.TotalEntries { + continue + } + entry, ok := getEntryByIndex(state, idx) + if !ok { + // Page not loaded yet; add placeholder + items = append(items, ui.ListItem{ + Text: "...", + Subtext: "Loading...", + }) + continue + } + item := entry.ToListItem() + item.Selected = (idx == state.SelectedIndex) + items = append(items, item) + } + return items +} diff --git a/internal/browser/layout_test.go b/internal/browser/layout_test.go index a5c5608..019c38c 100644 --- a/internal/browser/layout_test.go +++ b/internal/browser/layout_test.go @@ -14,7 +14,8 @@ func makeTestState(t *testing.T, entryCount int, scrollIndex int, visibleCount i t.Helper() s := NewBrowserState() s.CurrentPath = "/test/path" - s.ScrollIndex = scrollIndex + s.EntryHeight = 48.0 + s.ScrollOffset = float64(scrollIndex) * s.EntryHeight s.VisibleCount = visibleCount s.TotalEntries = entryCount @@ -62,7 +63,7 @@ func findListView(elements []ui.Element) (ui.ListView, bool) { func TestBrowserLayout_ElementCount(t *testing.T) { s := makeTestState(t, 200, 0, 20) - elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s) + elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s, nil) // Should produce: header + search bar + list view = 3 elements if len(elements) < 3 { @@ -75,7 +76,7 @@ func TestBrowserLayout_ElementCount(t *testing.T) { func TestBrowserLayout_HeaderRegion(t *testing.T) { s := makeTestState(t, 200, 0, 20) - elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s) + elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s, nil) // First element should be the header container if len(elements) == 0 { @@ -101,7 +102,7 @@ func TestBrowserLayout_HeaderRegion(t *testing.T) { func TestBrowserLayout_ListRegion(t *testing.T) { s := makeTestState(t, 200, 0, 20) - elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s) + elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s, nil) listView, found := findListView(elements) if !found { @@ -126,7 +127,7 @@ func TestBrowserLayout_ListRegion(t *testing.T) { func TestBrowserLayout_VisibleEntriesCount(t *testing.T) { s := makeTestState(t, 500, 100, 20) - elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s) + elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s, nil) listView, found := findListView(elements) if !found { @@ -148,7 +149,7 @@ func TestBrowserLayout_EmptyDirectory(t *testing.T) { s.CurrentPath = "/empty/dir" s.TotalEntries = 0 - elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s) + elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s, nil) if len(elements) < 3 { t.Errorf("expected at least 3 elements for empty dir, got %d", len(elements)) @@ -171,13 +172,14 @@ func TestBrowserLayout_SingleEntry(t *testing.T) { s.CurrentPath = "/test" s.TotalEntries = 1 s.VisibleCount = 20 + s.EntryHeight = 48.0 entries := []Entry{ NewEntry("/test/only.txt", "only.txt", 1024, time.Time{}, false), } s.Pages[0] = NewPage(0, entries) - elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s) + elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s, nil) listView, found := findListView(elements) if !found { @@ -195,7 +197,8 @@ func TestBrowserLayout_PartialPage(t *testing.T) { s := NewBrowserState() s.CurrentPath = "/test" s.TotalEntries = 150 // 1 full page + 50 in second page - s.ScrollIndex = 120 // Near the end + s.EntryHeight = 48.0 + s.ScrollOffset = 120 * 48 // Near the end s.VisibleCount = 20 entries := make([]Entry, 150) @@ -211,7 +214,7 @@ func TestBrowserLayout_PartialPage(t *testing.T) { s.Pages[0] = NewPage(0, entries[:100]) s.Pages[1] = NewPage(1, entries[100:]) - elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s) + elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s, nil) listView, found := findListView(elements) if !found { @@ -230,14 +233,15 @@ func TestBrowserLayout_UnloadedPage(t *testing.T) { s := NewBrowserState() s.CurrentPath = "/test" s.TotalEntries = 500 - s.ScrollIndex = 250 + s.EntryHeight = 48.0 + s.ScrollOffset = 250 * 48 s.VisibleCount = 20 // Only load page 0, not page 2 or 3 entries := make([]Entry, 100) s.Pages[0] = NewPage(0, entries) - elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s) + elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s, nil) // Layout should not crash with unloaded pages if len(elements) < 3 { @@ -258,7 +262,7 @@ func TestBrowserLayout_WithSearchQuery(t *testing.T) { } } - elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s) + elements := BrowserLayout(ui.Dp(800), ui.Dp(1200), s, nil) // Should still produce valid layout if len(elements) < 3 { @@ -274,7 +278,7 @@ func TestBrowserLayout_ScreenDimensions(t *testing.T) { // Test with different screen sizes for _, w := range []ui.Dp{400, 800, 1200} { for _, h := range []ui.Dp{600, 1200, 1800} { - elements := BrowserLayout(w, h, s) + elements := BrowserLayout(w, h, s, nil) if len(elements) < 3 { t.Errorf("layout failed for %dx%d: expected >= 3 elements, got %d", int(w), int(h), len(elements)) diff --git a/internal/browser/pixel_scroll_test.go b/internal/browser/pixel_scroll_test.go new file mode 100644 index 0000000..256218e --- /dev/null +++ b/internal/browser/pixel_scroll_test.go @@ -0,0 +1,258 @@ +package browser + +import ( + "testing" +) + +// --- Test: Per-Pixel Scroll Delta Applied Correctly --- + +func TestPixelScroll_DeltaApplied(t *testing.T) { + s := NewBrowserState() + s.TotalEntries = 1000 + s.VisibleCount = 20 + s.EntryHeight = 48.0 // pixels per entry + s.ScrollOffset = 0 + + // Scroll down by 100 pixels (should be precise, not rounded to entries) + HandlePixelScroll(s, 100) + + // ScrollOffset should be 100.0 (exact pixel delta) + expected := 100.0 + if s.ScrollOffset != expected { + t.Errorf("expected ScrollOffset=%.1f, got %.1f", expected, s.ScrollOffset) + } + + // Scroll up by 50 pixels + HandlePixelScroll(s, -50) + + expected = 50.0 + if s.ScrollOffset != expected { + t.Errorf("expected ScrollOffset=%.1f, got %.1f", expected, s.ScrollOffset) + } +} + +// --- Test: Per-Pixel Scroll Clamped to Minimum --- + +func TestPixelScroll_ClampMinimum(t *testing.T) { + s := NewBrowserState() + s.TotalEntries = 1000 + s.VisibleCount = 20 + s.EntryHeight = 48.0 + s.ScrollOffset = 0 + + // Try to scroll above the top + HandlePixelScroll(s, -100) + + // ScrollOffset should be clamped to 0 + if s.ScrollOffset != 0 { + t.Errorf("expected ScrollOffset=0 (clamped), got %.1f", s.ScrollOffset) + } +} + +// --- Test: Per-Pixel Scroll Clamped to Maximum --- + +func TestPixelScroll_ClampMaximum(t *testing.T) { + s := NewBrowserState() + s.TotalEntries = 1000 + s.VisibleCount = 20 + s.EntryHeight = 48.0 + s.ScrollOffset = 0 + + // Maximum scroll = (TotalEntries - VisibleCount) * EntryHeight + maxScroll := float64(s.TotalEntries-s.VisibleCount) * s.EntryHeight + + // Try to scroll past the bottom + HandlePixelScroll(s, int(maxScroll)+1000) + + // ScrollOffset should be clamped to max + if s.ScrollOffset != maxScroll { + t.Errorf("expected ScrollOffset=%.1f (clamped to max), got %.1f", maxScroll, s.ScrollOffset) + } +} + +// --- Test: Per-Pixel Scroll Preserves Sub-Entry Precision --- + +func TestPixelScroll_SubEntryPrecision(t *testing.T) { + s := NewBrowserState() + s.TotalEntries = 1000 + s.VisibleCount = 20 + s.EntryHeight = 48.0 + s.ScrollOffset = 0 + + // Scroll by a value that doesn't align to entry boundaries + // 73 pixels = 1 full entry (48px) + 25 pixels into the next entry + HandlePixelScroll(s, 73) + + expected := 73.0 + if s.ScrollOffset != expected { + t.Errorf("expected ScrollOffset=%.1f (sub-entry precision), got %.1f", expected, s.ScrollOffset) + } + + // Scroll another 100 pixels (total 173 = 3 entries + 29px) + HandlePixelScroll(s, 100) + + expected = 173.0 + if s.ScrollOffset != expected { + t.Errorf("expected ScrollOffset=%.1f (accumulated sub-entry precision), got %.1f", expected, s.ScrollOffset) + } +} + +// --- Test: Pixel Scroll Index Derived from Offset --- + +func TestPixelScroll_IndexFromOffset(t *testing.T) { + s := NewBrowserState() + s.TotalEntries = 1000 + s.VisibleCount = 20 + s.EntryHeight = 48.0 + s.ScrollOffset = 0 + + // Initial index should be 0 + if s.GetScrollIndex() != 0 { + t.Errorf("expected initial ScrollIndex=0, got %d", s.GetScrollIndex()) + } + + // Scroll down by 100 pixels + HandlePixelScroll(s, 100) + + // ScrollIndex should be floor(100 / 48) = 2 + expectedIndex := 2 + if s.GetScrollIndex() != expectedIndex { + t.Errorf("expected ScrollIndex=%d (from offset 100 / height 48), got %d", expectedIndex, s.GetScrollIndex()) + } + + // Scroll up by 50 pixels (offset now 50) + HandlePixelScroll(s, -50) + + // ScrollIndex should be floor(50 / 48) = 1 + expectedIndex = 1 + if s.GetScrollIndex() != expectedIndex { + t.Errorf("expected ScrollIndex=%d (from offset 50 / height 48), got %d", expectedIndex, s.GetScrollIndex()) + } +} + +// --- Test: Pixel Scroll Eviction Triggered --- + +func TestPixelScroll_EvictionTriggered(t *testing.T) { + s := NewBrowserState() + s.TotalEntries = 5000 + s.VisibleCount = 20 + s.EntryHeight = 48.0 + s.ScrollOffset = 0 + + // Load pages at various positions + entries := make([]Entry, 5000) + for i := 0; i < 5000; i++ { + entries[i] = NewEntry("/test/file.txt", "file.txt", 100, s.TapTimestamp, false) + } + s.SortIndex = &DirectoryIndex{ + Entries: entries, + SortOrders: map[string][]int{"name_asc": buildPositionMap(entries, SortModeNameAsc)}, + } + for pageIdx := 0; pageIdx <= 10; pageIdx++ { + p := loadPageFromIndex(s, pageIdx) + if p != nil { + s.Pages[pageIdx] = p + } + } + + // Scroll far away using pixel offset + // Scroll to page 40+ (40 * 100 * 48 = 192000 pixels) + HandlePixelScroll(s, 192000) + + // Pages far from current position should be evicted + // Check that page 0 was evicted (it's far from page 40) + if page, exists := s.Pages[0]; exists && page.Loaded { + t.Error("expected page 0 to be evicted after scrolling far away") + } +} + +// --- Test: Pixel Scroll Prefetch Triggered --- + +func TestPixelScroll_PrefetchTriggered(t *testing.T) { + s := NewBrowserState() + s.TotalEntries = 2000 + s.VisibleCount = 20 + s.EntryHeight = 48.0 + s.ScrollOffset = 0 + + // Load some pages around current position + entries := make([]Entry, 2000) + for i := 0; i < 2000; i++ { + entries[i] = NewEntry("/test/file.txt", "file.txt", 100, s.TapTimestamp, false) + } + s.SortIndex = &DirectoryIndex{ + Entries: entries, + SortOrders: map[string][]int{"name_asc": buildPositionMap(entries, SortModeNameAsc)}, + } + for pageIdx := 3; pageIdx <= 7; pageIdx++ { + p := loadPageFromIndex(s, pageIdx) + if p != nil { + s.Pages[pageIdx] = p + } + } + + // Record current page count + pagesBefore := len(s.Pages) + + // Scroll to trigger prefetch using pixel offset + HandlePixelScroll(s, 200*48) // Scroll to page ~7 + + // After scroll, prefetch should have been triggered + // Page count may have increased due to prefetch + if len(s.Pages) < pagesBefore { + t.Errorf("expected page count to not decrease after scroll, before=%d, after=%d", pagesBefore, len(s.Pages)) + } +} + +// --- Test: Pixel Scroll With Small Directory --- + +func TestPixelScroll_SmallDirectory(t *testing.T) { + s := NewBrowserState() + s.TotalEntries = 10 // Smaller than VisibleCount + s.VisibleCount = 20 + s.EntryHeight = 48.0 + s.ScrollOffset = 0 + + // Try to scroll down (should be clamped since no scrollable area) + HandlePixelScroll(s, 1000) + + // ScrollOffset should be 0 (can't scroll beyond available entries) + if s.ScrollOffset != 0 { + t.Errorf("expected ScrollOffset=0 for small directory, got %.1f", s.ScrollOffset) + } +} + +// --- Test: Pixel Scroll To Beginning --- + +func TestPixelScroll_ToBeginning(t *testing.T) { + s := NewBrowserState() + s.TotalEntries = 1000 + s.VisibleCount = 20 + s.EntryHeight = 48.0 + s.ScrollOffset = 500 * 48 // Somewhere in the middle + + // Scroll all the way to the beginning + HandlePixelScroll(s, -100000) + + if s.ScrollOffset != 0 { + t.Errorf("expected ScrollOffset=0 at beginning, got %.1f", s.ScrollOffset) + } +} + +// --- Test: Pixel Scroll To End --- + +func TestPixelScroll_ToEnd(t *testing.T) { + s := NewBrowserState() + s.TotalEntries = 1000 + s.VisibleCount = 20 + s.EntryHeight = 48.0 + s.ScrollOffset = 0 + + // Scroll all the way to the end + maxScroll := float64(s.TotalEntries-s.VisibleCount) * s.EntryHeight + HandlePixelScroll(s, int(maxScroll)+1000) + + if s.ScrollOffset != maxScroll { + t.Errorf("expected ScrollOffset=%.1f at end, got %.1f", maxScroll, s.ScrollOffset) + } +} diff --git a/internal/browser/scroll_test.go b/internal/browser/scroll_test.go index 520fd71..06278d5 100644 --- a/internal/browser/scroll_test.go +++ b/internal/browser/scroll_test.go @@ -10,16 +10,15 @@ func TestScrollClamp_Minimum(t *testing.T) { s := NewBrowserState() s.TotalEntries = 500 s.VisibleCount = 20 + s.EntryHeight = 48.0 + s.ScrollOffset = 0 - // Set scroll index below minimum - s.ScrollIndex = -10 + // Try to scroll above the top + HandlePixelScroll(s, -5*48) - // Apply scroll clamp - HandleScroll(s, -5) - - // ScrollIndex should be clamped to 0 - if s.ScrollIndex != 0 { - t.Errorf("expected ScrollIndex=0 (clamped), got %d", s.ScrollIndex) + // ScrollOffset should be clamped to 0 + if s.ScrollOffset != 0 { + t.Errorf("expected ScrollOffset=0 (clamped), got %.1f", s.ScrollOffset) } } @@ -29,17 +28,16 @@ func TestScrollClamp_Maximum(t *testing.T) { s := NewBrowserState() s.TotalEntries = 500 s.VisibleCount = 20 + s.EntryHeight = 48.0 + s.ScrollOffset = 0 - // Set scroll index above maximum - s.ScrollIndex = 1000 + // Scroll past the maximum to trigger clamp + maxScroll := float64(s.TotalEntries-s.VisibleCount) * s.EntryHeight + HandlePixelScroll(s, int(maxScroll)+1000) - // Apply scroll clamp - HandleScroll(s, 100) - - // ScrollIndex should be clamped to max (TotalEntries - VisibleCount) - maxScroll := s.TotalEntries - s.VisibleCount - if s.ScrollIndex != maxScroll { - t.Errorf("expected ScrollIndex=%d (clamped to max), got %d", maxScroll, s.ScrollIndex) + // ScrollOffset should be clamped to max (TotalEntries - VisibleCount) * EntryHeight + if s.ScrollOffset != maxScroll { + t.Errorf("expected ScrollOffset=%.1f (clamped to max), got %.1f", maxScroll, s.ScrollOffset) } } @@ -49,20 +47,23 @@ func TestScrollDelta_Computation(t *testing.T) { s := NewBrowserState() s.TotalEntries = 1000 s.VisibleCount = 20 - s.ScrollIndex = 100 + s.EntryHeight = 48.0 + s.ScrollOffset = 100 * 48 // Scroll down by 5 entries (positive delta) - HandleScroll(s, 5) + HandlePixelScroll(s, 5*48) - if s.ScrollIndex != 105 { - t.Errorf("expected ScrollIndex=105 after scrolling down 5, got %d", s.ScrollIndex) + expectedOffset := float64(105 * 48) + if s.ScrollOffset != expectedOffset { + t.Errorf("expected ScrollOffset=%.1f after scrolling down 5 entries, got %.1f", expectedOffset, s.ScrollOffset) } // Scroll up by 3 entries (negative delta) - HandleScroll(s, -3) + HandlePixelScroll(s, -3*48) - if s.ScrollIndex != 102 { - t.Errorf("expected ScrollIndex=102 after scrolling up 3, got %d", s.ScrollIndex) + expectedOffset = float64(102 * 48) + if s.ScrollOffset != expectedOffset { + t.Errorf("expected ScrollOffset=%.1f after scrolling up 3 entries, got %.1f", expectedOffset, s.ScrollOffset) } } @@ -72,7 +73,8 @@ func TestScroll_PrefetchTriggered(t *testing.T) { s := NewBrowserState() s.TotalEntries = 2000 s.VisibleCount = 20 - s.ScrollIndex = 500 // Page 5 + s.EntryHeight = 48.0 + s.ScrollOffset = 500 * 48 // Page 5 // Load some pages around current position entries := make([]Entry, 2000) @@ -95,13 +97,13 @@ func TestScroll_PrefetchTriggered(t *testing.T) { pagesBefore := len(s.Pages) // Scroll to trigger prefetch - HandleScroll(s, 200) // Scroll to page 7+ + HandlePixelScroll(s, 200*48) // Scroll to page 7+ // After scroll, prefetch should have been triggered // (exact behavior depends on implementation) // For now, verify that scroll happened correctly - if s.ScrollIndex <= 500 { - t.Errorf("expected ScrollIndex > 500 after scrolling, got %d", s.ScrollIndex) + if s.GetScrollIndex() <= 500 { + t.Errorf("expected GetScrollIndex() > 500 after scrolling, got %d", s.GetScrollIndex()) } // Page count may have increased due to prefetch @@ -116,7 +118,8 @@ func TestScroll_EvictionTriggered(t *testing.T) { s := NewBrowserState() s.TotalEntries = 5000 s.VisibleCount = 20 - s.ScrollIndex = 0 + s.EntryHeight = 48.0 + s.ScrollOffset = 0 // Load pages at various positions entries := make([]Entry, 5000) @@ -136,7 +139,7 @@ func TestScroll_EvictionTriggered(t *testing.T) { } // Scroll far away from initial position - HandleScroll(s, 4000) // Scroll to page 40+ + HandlePixelScroll(s, 4000*48) // Scroll to page 40+ // Pages far from current position should be evicted // Check that page 0 was evicted (it's far from page 40) @@ -151,16 +154,18 @@ func TestScroll_WithSearchResults(t *testing.T) { s := NewBrowserState() s.TotalEntries = 100 s.VisibleCount = 20 - s.ScrollIndex = 0 + s.EntryHeight = 48.0 + s.ScrollOffset = 0 // Set up search results s.SearchResults = []int{5, 15, 25, 35, 45} // Scroll should work normally even with search results - HandleScroll(s, 10) + HandlePixelScroll(s, 10*48) - if s.ScrollIndex != 10 { - t.Errorf("expected ScrollIndex=10, got %d", s.ScrollIndex) + expectedOffset := float64(10 * 48) + if s.ScrollOffset != expectedOffset { + t.Errorf("expected ScrollOffset=%.1f, got %.1f", expectedOffset, s.ScrollOffset) } } @@ -170,13 +175,14 @@ func TestScroll_ToBeginning(t *testing.T) { s := NewBrowserState() s.TotalEntries = 1000 s.VisibleCount = 20 - s.ScrollIndex = 500 + s.EntryHeight = 48.0 + s.ScrollOffset = 500 * 48 // Scroll all the way to the beginning - HandleScroll(s, -500) + HandlePixelScroll(s, -100000) - if s.ScrollIndex != 0 { - t.Errorf("expected ScrollIndex=0 at beginning, got %d", s.ScrollIndex) + if s.ScrollOffset != 0 { + t.Errorf("expected ScrollOffset=0 at beginning, got %.1f", s.ScrollOffset) } } @@ -186,14 +192,14 @@ func TestScroll_ToEnd(t *testing.T) { s := NewBrowserState() s.TotalEntries = 1000 s.VisibleCount = 20 - s.ScrollIndex = 0 + s.EntryHeight = 48.0 + s.ScrollOffset = 0 // Scroll all the way to the end - HandleScroll(s, 1000) - - maxScroll := s.TotalEntries - s.VisibleCount - if s.ScrollIndex != maxScroll { - t.Errorf("expected ScrollIndex=%d at end, got %d", maxScroll, s.ScrollIndex) + maxScroll := float64(s.TotalEntries-s.VisibleCount) * s.EntryHeight + HandlePixelScroll(s, int(maxScroll)+1000) + if s.ScrollOffset != maxScroll { + t.Errorf("expected ScrollOffset=%.1f at end, got %.1f", maxScroll, s.ScrollOffset) } } @@ -203,14 +209,15 @@ func TestScroll_SmallDirectory(t *testing.T) { s := NewBrowserState() s.TotalEntries = 10 // Smaller than VisibleCount s.VisibleCount = 20 - s.ScrollIndex = 0 + s.EntryHeight = 48.0 + s.ScrollOffset = 0 // Try to scroll down (should be clamped) - HandleScroll(s, 100) + HandlePixelScroll(s, 1000) - // ScrollIndex should be 0 (can't scroll beyond available entries) - if s.ScrollIndex != 0 { - t.Errorf("expected ScrollIndex=0 for small directory, got %d", s.ScrollIndex) + // ScrollOffset should be 0 (can't scroll beyond available entries) + if s.ScrollOffset != 0 { + t.Errorf("expected ScrollOffset=0 for small directory, got %.1f", s.ScrollOffset) } } @@ -220,7 +227,8 @@ func TestScroll_PrefetchTriggeredOnPageBoundary(t *testing.T) { s := NewBrowserState() s.TotalEntries = 500 s.VisibleCount = 20 - s.ScrollIndex = 90 // Near page boundary (page 0 ends at 100) + s.EntryHeight = 48.0 + s.ScrollOffset = 90 * 48 // Near page boundary (page 0 ends at 100) // Load page 0 entries := make([]Entry, 500) @@ -235,10 +243,10 @@ func TestScroll_PrefetchTriggeredOnPageBoundary(t *testing.T) { s.Pages[0] = loadPageFromIndex(s, 0) // Scroll across page boundary - HandleScroll(s, 15) // Should cross into page 1 + HandlePixelScroll(s, 15*48) // Should cross into page 1 // Verify scroll happened - if s.ScrollIndex != 105 { - t.Errorf("expected ScrollIndex=105, got %d", s.ScrollIndex) + if s.GetScrollIndex() != 105 { + t.Errorf("expected GetScrollIndex()=105, got %d", s.GetScrollIndex()) } } diff --git a/internal/browser/search.go b/internal/browser/search.go index ce44b0e..a85656a 100644 --- a/internal/browser/search.go +++ b/internal/browser/search.go @@ -5,11 +5,11 @@ import "strings" // HandleSearch filters entries by the given query string. // It performs case-insensitive substring matching across all loaded pages. // When query is empty, search results are cleared. -// When results are found, ScrollIndex is set to the first match. +// When results are found, ScrollOffset is set to jump to the first match. func HandleSearch(s *BrowserState, query string) { s.Query = query - // Empty query clears search results; leave ScrollIndex unchanged. + // Empty query clears search results; leave ScrollOffset unchanged. if query == "" { s.SearchResults = nil return @@ -39,6 +39,6 @@ func HandleSearch(s *BrowserState, query string) { // Jump to first result if any matches found. if len(results) > 0 { - s.ScrollIndex = results[0] + s.ScrollOffset = float64(results[0]) * s.EntryHeight } } diff --git a/internal/browser/search_test.go b/internal/browser/search_test.go index 2416d0f..3ca4bf9 100644 --- a/internal/browser/search_test.go +++ b/internal/browser/search_test.go @@ -14,6 +14,7 @@ func makeSearchState(t *testing.T, entries []Entry) *BrowserState { s.TotalEntries = len(entries) s.VisibleCount = 20 s.CurrentPath = "/test" + s.EntryHeight = 48.0 // Distribute entries across pages for pageIdx := 0; pageIdx <= (len(entries)-1)/PageSize; pageIdx++ { @@ -51,8 +52,8 @@ func TestSearch_EmptyQuery(t *testing.T) { t.Errorf("expected SearchResults to be nil after empty query, got %v", s.SearchResults) } // Scroll should not change when clearing search - if s.ScrollIndex != 0 { - t.Errorf("expected ScrollIndex=0, got %d", s.ScrollIndex) + if s.GetScrollIndex() != 0 { + t.Errorf("expected ScrollIndex=0, got %d", s.GetScrollIndex()) } } @@ -145,8 +146,8 @@ func TestSearch_NoMatch(t *testing.T) { t.Errorf("expected 0 results for 'zzz', got %d: %v", len(s.SearchResults), s.SearchResults) } // Scroll should not change when there are no results - if s.ScrollIndex != 0 { - t.Errorf("expected ScrollIndex=0 when no match, got %d", s.ScrollIndex) + if s.GetScrollIndex() != 0 { + t.Errorf("expected ScrollIndex=0 when no match, got %d", s.GetScrollIndex()) } } @@ -162,7 +163,7 @@ func TestSearch_JumpToFirst(t *testing.T) { }) // Start with scroll at position 0 - s.ScrollIndex = 0 + s.ScrollOffset = 0 // Search for "delta" — should jump to index 3 HandleSearch(s, "delta") @@ -170,8 +171,8 @@ func TestSearch_JumpToFirst(t *testing.T) { if len(s.SearchResults) != 1 { t.Fatalf("expected 1 result for 'delta', got %d", len(s.SearchResults)) } - if s.ScrollIndex != 3 { - t.Errorf("expected ScrollIndex=3 (jumped to first match), got %d", s.ScrollIndex) + if s.GetScrollIndex() != 3 { + t.Errorf("expected ScrollIndex=3 (jumped to first match), got %d", s.GetScrollIndex()) } // Verify query is stored @@ -406,8 +407,8 @@ func TestSearch_EmptyDirectory(t *testing.T) { if len(s.SearchResults) != 0 { t.Errorf("expected 0 results for empty directory, got %d", len(s.SearchResults)) } - if s.ScrollIndex != 0 { - t.Errorf("expected ScrollIndex=0, got %d", s.ScrollIndex) + if s.GetScrollIndex() != 0 { + t.Errorf("expected ScrollIndex=0, got %d", s.GetScrollIndex()) } } diff --git a/internal/browser/types.go b/internal/browser/types.go index 6d2582a..f008f02 100644 --- a/internal/browser/types.go +++ b/internal/browser/types.go @@ -29,9 +29,13 @@ type Page struct { // BrowserState holds all mutable browser state owned by the logic goroutine. type BrowserState struct { // Navigation - CurrentPath string // Currently browsed directory (relative to root) - ScrollIndex int // Index of first visible entry (not pixel offset) - VisibleCount int // Number of entries currently visible + CurrentPath string // Currently browsed directory (relative to root) + ScrollOffset float64 // Vertical scroll offset in pixels (per-pixel scrolling) + EntryHeight float64 // Height of a single entry in pixels + VisibleCount int // Number of entries currently visible + + // Computed from ScrollOffset and EntryHeight + scrollIndex int // Index of first visible entry (derived, not persisted) // Lazy loading Pages map[int]*Page // Loaded pages by page index @@ -88,6 +92,7 @@ func NewBrowserState() *BrowserState { LetterOffsets: make(map[string]int), SelectedIndex: -1, SortMode: SortModeNameAsc, // Default sort mode + EntryHeight: 48.0, } } @@ -119,8 +124,8 @@ func formatSize(bytes int64) string { // EvictPages removes pages that are far from the current scroll position. func (s *BrowserState) EvictPages() { - minPage := s.ScrollIndex/PageSize - PrefetchDist - maxPage := (s.ScrollIndex + s.VisibleCount)/PageSize + PrefetchDist + minPage := s.GetScrollIndex()/PageSize - PrefetchDist + maxPage := (s.GetScrollIndex() + s.VisibleCount)/PageSize + PrefetchDist for idx, page := range s.Pages { if idx < minPage || idx > maxPage { page.Entries = nil // Release memory @@ -129,10 +134,19 @@ func (s *BrowserState) EvictPages() { } } +// GetScrollIndex returns the index of the first visible entry, derived from +// the per-pixel ScrollOffset and EntryHeight. +func (s *BrowserState) GetScrollIndex() int { + if s.EntryHeight <= 0 { + return 0 + } + return int(s.ScrollOffset / s.EntryHeight) +} + // Reset clears all browser state for navigation. func (s *BrowserState) Reset() { s.Pages = make(map[int]*Page) - s.ScrollIndex = 0 + s.ScrollOffset = 0 s.SelectedIndex = -1 s.SearchResults = nil s.Query = "" diff --git a/internal/browser/types_test.go b/internal/browser/types_test.go index fa4c186..e83dd33 100644 --- a/internal/browser/types_test.go +++ b/internal/browser/types_test.go @@ -58,9 +58,10 @@ func TestPageSizeConstant(t *testing.T) { func TestNewBrowserState(t *testing.T) { s := NewBrowserState() + s.EntryHeight = 48.0 - if s.ScrollIndex != 0 { - t.Errorf("expected ScrollIndex=0, got %d", s.ScrollIndex) + if s.GetScrollIndex() != 0 { + t.Errorf("expected ScrollIndex=0, got %d", s.GetScrollIndex()) } if s.SelectedIndex != -1 { t.Errorf("expected SelectedIndex=-1, got %d", s.SelectedIndex) @@ -116,7 +117,8 @@ func TestFormatSize(t *testing.T) { func TestBrowserStateEvictPages(t *testing.T) { s := NewBrowserState() - s.ScrollIndex = 500 // Page 5 + s.EntryHeight = 48.0 + s.ScrollOffset = 500 * 48 // Page 5 s.VisibleCount = 10 // With ScrollIndex=500, PageSize=100: @@ -152,7 +154,8 @@ func TestBrowserStateEvictPages(t *testing.T) { func TestBrowserStateReset(t *testing.T) { s := NewBrowserState() - s.ScrollIndex = 100 + s.EntryHeight = 48.0 + s.ScrollOffset = 100 * 48 s.SelectedIndex = 5 s.Pages[0] = NewPage(0, nil) s.Query = "test" @@ -160,8 +163,8 @@ func TestBrowserStateReset(t *testing.T) { s.Reset() - if s.ScrollIndex != 0 { - t.Errorf("expected ScrollIndex=0, got %d", s.ScrollIndex) + if s.GetScrollIndex() != 0 { + t.Errorf("expected ScrollIndex=0, got %d", s.GetScrollIndex()) } if s.SelectedIndex != -1 { t.Errorf("expected SelectedIndex=-1, got %d", s.SelectedIndex) diff --git a/internal/editor/state.go b/internal/editor/state.go index e325b78..647d8c0 100644 --- a/internal/editor/state.go +++ b/internal/editor/state.go @@ -100,6 +100,7 @@ func (s *State) layout() []ui.Element { dpW := ui.ToDp(ui.Px(s.PixelWidth), s.scale) dpH := ui.ToDp(ui.Px(s.PixelHeight), s.scale) + // Calculate VisibleCount before laying out the browser page. // This ensures the browser shows entries based on the current viewport. if s.PixelHeight > 0 && s.scale > 0 { @@ -139,11 +140,11 @@ func HandleScroll(data any) { } } -// HandleBrowserScroll updates the browser scroll index by the given delta. -// Delegates to the browser package's handler. +// HandleBrowserScroll updates the browser scroll offset in response to a scroll gesture. +// The delta is in pixels (from gesture.Scroll.Update). Delegates to the browser package. func HandleBrowserScroll(data any) { - delta := data.(int) // entries - browser.HandleScroll(&TheState.Browser, delta) + delta := data.(int) // pixel delta from gesture.Scroll.Update + browser.HandlePixelScroll(&TheState.Browser, delta) } // GoToBrowser switches the app to the browser page. @@ -168,11 +169,13 @@ func ToggleSortOrder(data any) { // Cycle through the 4 sort modes TheState.Browser.SortMode = (TheState.Browser.SortMode + 1) % 4 // Reset scroll on sort change - TheState.Browser.ScrollIndex = 0 + TheState.Browser.ScrollOffset = 0 // Clear cached pages so they reload with the new sort order TheState.Browser.Pages = make(map[int]*browser.Page) // Reload initial pages with the new sort order browser.LoadInitialPages(&TheState.Browser) + // Recompute search results if there's an active search query + browser.HandleSortModeChange(&TheState.Browser) } // EditorLayout computes the element tree for the editor page. diff --git a/internal/test/e2e/browser_files_test.go b/internal/test/e2e/browser_files_test.go index ce0e929..7263522 100644 --- a/internal/test/e2e/browser_files_test.go +++ b/internal/test/e2e/browser_files_test.go @@ -68,10 +68,10 @@ func TestBrowserFilesVisible(t *testing.T) { // The mock filesystem has 5 root-level files and 6 directories = 11 entries if len(listView.Items) == 0 { t.Errorf("browser ListView has 0 items — files from mock filesystem not showing up") - t.Logf("BrowserState: TotalEntries=%d, VisibleCount=%d, ScrollIndex=%d", + t.Logf("BrowserState: TotalEntries=%d, VisibleCount=%d, ScrollOffset=%.1f", state.Browser.TotalEntries, state.Browser.VisibleCount, - state.Browser.ScrollIndex) + state.Browser.ScrollOffset) } // Verify some known files are present diff --git a/internal/test/e2e/search_test.go b/internal/test/e2e/search_test.go new file mode 100644 index 0000000..1d86c39 --- /dev/null +++ b/internal/test/e2e/search_test.go @@ -0,0 +1,284 @@ +package e2e_test + +import ( + "slices" + "strings" + "testing" + "time" + + "pad/internal/browser" + "pad/internal/editor" + "pad/internal/test/e2e" +) + +// TestSearchFiltersList verifies that typing in the search box filters the +// browser list so that only matching entries appear. +// +// BUG: computeVisibleEntries does not check SearchResults; it shows all +// entries in the visible window regardless of the search query. +func TestSearchFiltersList(t *testing.T) { + h := e2e.NewHarnessWithDefaults() + defer h.Cleanup() + + _, err := h.WaitForFrameCount(1, 5*time.Second) + if err != nil { + t.Fatalf("timeout waiting for initial frames: %v", err) + } + + // Navigate to browser page + editor.GoToBrowser(nil) + h.SendConfig(780, 1688) + time.Sleep(200 * time.Millisecond) + + // Get initial list items (no filter) + initialItems := getListItems(t, e2e.GetLastFrame(h)) + t.Logf("Initial items (%d): %v", len(initialItems), initialItems) + + if len(initialItems) == 0 { + t.Skip("no list items; skipping") + } + + // Search for "doc" — should match: Documents (directory) + // config.yaml does NOT contain "doc" + query := "doc" + h.SendSearchQuery(query) + time.Sleep(200 * time.Millisecond) + + // Wait for new frame + _, err = h.WaitForFrameCount(h.FrameCount()+1, 5*time.Second) + if err != nil { + t.Logf("no new frame after search, checking current frame") + } + + searchedItems := getListItems(t, e2e.GetLastFrame(h)) + t.Logf("After searching '%s' (%d items): %v", query, len(searchedItems), searchedItems) + + // All visible items should contain the query (case-insensitive) + for _, item := range searchedItems { + if item == "..." { + continue // loading placeholder + } + if !strings.Contains(strings.ToLower(item), strings.ToLower(query)) { + t.Errorf("search bug: item %q does not contain '%s' but was shown in search results", item, query) + } + } + + // Documents should be in the results + foundDocuments := false + for _, item := range searchedItems { + if item == "Documents" { + foundDocuments = true + break + } + } + if !foundDocuments && len(searchedItems) > 0 { + t.Errorf("search bug: 'Documents' should be in search results for query '%s'", query) + } +} + +// TestSearchWithSortModeChange verifies that changing sort mode while +// a search query is active preserves the filter. +// +// BUG: ToggleSortOrder clears pages but doesn't recompute SearchResults. +// After a sort change, stale SearchResults indices point to different entries. +func TestSearchWithSortModeChange(t *testing.T) { + h := e2e.NewHarnessWithDefaults() + defer h.Cleanup() + + _, err := h.WaitForFrameCount(1, 5*time.Second) + if err != nil { + t.Fatalf("timeout waiting for initial frames: %v", err) + } + + // Navigate to browser page + editor.GoToBrowser(nil) + h.SendConfig(780, 1688) + time.Sleep(200 * time.Millisecond) + + // Apply a search query + query := "doc" + h.SendSearchQuery(query) + time.Sleep(200 * time.Millisecond) + + _, err = h.WaitForFrameCount(h.FrameCount()+1, 5*time.Second) + if err != nil { + t.Logf("no new frame after search, checking current frame") + } + + afterSearchItems := getListItems(t, e2e.GetLastFrame(h)) + t.Logf("After search '%s' (%d items): %v", query, len(afterSearchItems), afterSearchItems) + + // Verify all items contain the query + for _, item := range afterSearchItems { + if item == "..." { + continue + } + if !strings.Contains(strings.ToLower(item), strings.ToLower(query)) { + t.Errorf("search bug: item %q does not contain '%s' but was shown", item, query) + } + } + + // Now toggle sort mode WHILE search is active + editor.ToggleSortOrder(nil) + h.SendConfig(780, 1688) + time.Sleep(300 * time.Millisecond) + + afterSortItems := getListItems(t, e2e.GetLastFrame(h)) + t.Logf("After sort toggle (%d items): %v", len(afterSortItems), afterSortItems) + + // After sort toggle, the items should STILL be filtered by the search query + for _, item := range afterSortItems { + if item == "..." { + continue + } + if !strings.Contains(strings.ToLower(item), strings.ToLower(query)) { + t.Errorf("BUG: after sort toggle with active search, item %q does not contain '%s'. Search filter was lost!", item, query) + } + } +} + +// TestSearchDirect verifies HandleSearch computes correct indices. +func TestSearchDirect(t *testing.T) { + state := browser.NewBrowserState() + state.CurrentPath = "/" + state.VisibleCount = 30 + + // Create entries with known names + entries := []browser.Entry{ + browser.NewEntry("/alpha.txt", "alpha.txt", 100, time.Now(), false), + browser.NewEntry("/beta.txt", "beta.txt", 200, time.Now(), false), + browser.NewEntry("/gamma.doc", "gamma.doc", 300, time.Now(), false), + browser.NewEntry("/delta.log", "delta.log", 400, time.Now(), false), + browser.NewEntry("/epsilon.txt", "epsilon.txt", 500, time.Now(), false), + browser.NewEntry("/zeta.doc", "zeta.doc", 600, time.Now(), false), + } + + state.TotalEntries = len(entries) + state.SortIndex = &browser.DirectoryIndex{ + Path: "/", + EntryCount: len(entries), + Entries: entries, + SortOrders: map[string][]int{ + "name_asc": {0, 1, 2, 3, 4, 5}, + }, + } + state.SortMode = browser.SortModeNameAsc + + // Load initial pages + browser.LoadInitialPages(state) + + // Search for "txt" — should match: alpha.txt, beta.txt, epsilon.txt + browser.HandleSearch(state, "txt") + + expectedMatches := 3 // alpha.txt, beta.txt, epsilon.txt + if len(state.SearchResults) != expectedMatches { + t.Errorf("expected %d search results, got %d: %v", + expectedMatches, len(state.SearchResults), state.SearchResults) + } + + // Verify each result index points to a matching entry + for _, idx := range state.SearchResults { + if idx >= len(entries) { + t.Errorf("search result index %d out of bounds (total: %d)", idx, len(entries)) + continue + } + entry := entries[idx] + if !strings.Contains(strings.ToLower(entry.Name), "txt") { + t.Errorf("search result index %d points to %q which does not contain 'txt'", idx, entry.Name) + } + } +} + +// TestSortModeChangePreservesSearchFilter verifies at the state level that +// changing sort mode with an active search query maintains correct filtering. +func TestSortModeChangePreservesSearchFilter(t *testing.T) { + h := e2e.NewHarnessWithDefaults() + defer h.Cleanup() + + _, err := h.WaitForFrameCount(1, 5*time.Second) + if err != nil { + t.Fatalf("timeout waiting for initial frames: %v", err) + } + + state := h.State() + + // Navigate to browser + editor.GoToBrowser(nil) + h.SendConfig(780, 1688) + time.Sleep(200 * time.Millisecond) + + // Apply search + query := "doc" + h.SendSearchQuery(query) + time.Sleep(200 * time.Millisecond) + + // Record search results before sort change + searchResultsBefore := make([]int, len(state.Browser.SearchResults)) + copy(searchResultsBefore, state.Browser.SearchResults) + t.Logf("Search results before sort change: %v", searchResultsBefore) + + // Verify all pre-sort results are valid matches + for _, idx := range state.Browser.SearchResults { + if idx < len(state.Browser.SortIndex.Entries) { + entry := state.Browser.SortIndex.Entries[idx] + if !strings.Contains(strings.ToLower(entry.Name), strings.ToLower(query)) { + t.Errorf("pre-sort: index %d -> %q does not match query %q", idx, entry.Name, query) + } + } + } + + // Toggle sort mode + editor.ToggleSortOrder(nil) + h.SendConfig(780, 1688) + time.Sleep(300 * time.Millisecond) + + // After sort change, search results should still be valid + for _, idx := range state.Browser.SearchResults { + if idx >= len(state.Browser.SortIndex.Entries) { + t.Errorf("search result index %d is out of bounds (total: %d)", + idx, state.Browser.TotalEntries) + continue + } + entry := state.Browser.SortIndex.Entries[idx] + if !strings.Contains(strings.ToLower(entry.Name), strings.ToLower(query)) { + t.Errorf("post-sort: search result index %d -> %q does not match query %q", + idx, entry.Name, query) + } + } +} + +// TestSearchClearRestoresFullList verifies that clearing the search query +// restores the full unfiltered list. +func TestSearchClearRestoresFullList(t *testing.T) { + h := e2e.NewHarnessWithDefaults() + defer h.Cleanup() + + _, err := h.WaitForFrameCount(1, 5*time.Second) + if err != nil { + t.Fatalf("timeout waiting for initial frames: %v", err) + } + + // Navigate to browser page + editor.GoToBrowser(nil) + h.SendConfig(780, 1688) + time.Sleep(200 * time.Millisecond) + + initialItems := getListItems(t, e2e.GetLastFrame(h)) + t.Logf("Initial items (%d): %v", len(initialItems), initialItems) + + // Search for something specific + h.SendSearchQuery("doc") + time.Sleep(200 * time.Millisecond) + + // Clear search + h.SendSearchQuery("") + time.Sleep(200 * time.Millisecond) + + clearedItems := getListItems(t, e2e.GetLastFrame(h)) + t.Logf("After clear (%d items): %v", len(clearedItems), clearedItems) + + // After clearing, should show all entries again + if !slices.Equal(initialItems, clearedItems) { + t.Errorf("after clearing search, list should match initial list.\n initial: %v\n cleared: %v", initialItems, clearedItems) + } +} diff --git a/internal/ui/render.go b/internal/ui/render.go index ea36180..775d591 100644 --- a/internal/ui/render.go +++ b/internal/ui/render.go @@ -3,6 +3,7 @@ package ui import ( "bytes" "embed" + "fmt" "image" "image/color" _ "image/png" @@ -199,6 +200,7 @@ func (r *Renderer) CheckGestures(q input.Source, m unit.Metric) []InputEvent { delta := reg.scroll.Update(m, q, time.Now(), gesture.Vertical, pointer.ScrollRange{}, pointer.ScrollRange{Min: -(1<<30), Max: 1<<30}) if delta != 0 { + fmt.Printf("Scroll detected: %v\n", delta) events = append(events, InputEvent{ Handler: reg.handler, Data: delta,