Merge task: UI polish: ListBox Padding scroll clip, 4s log rebuild, ClassifyItems side effects

This commit is contained in:
mika kuns
2026-08-11 18:34:36 +02:00
6 changed files with 303 additions and 13 deletions
+1 -1
View File
@@ -111,7 +111,7 @@ snaps `CanResize="True"` windows, which is the opt-in), and it insets itself by
- **`PathIcon` *fills* its geometry.** Line-art/stroke icons must be authored as filled geometry or rendered with a stroked `Path` (e.g. `Icon.PlanDay` via the `Path.plan-icon` style). A pure stroke path in a `PathIcon` is **invisible**.
- **`NumericUpDown.Value` is `decimal?` and goes null while the box is empty** — i.e. every time the user clears a value to type a new one. Bound TwoWay to a non-nullable `int`/`decimal`, that null throws `InvalidCastException`. Either bind a `decimal?` property (as `AgentConfigEditorViewModel.MaxTurns` does) or add `Converter={StaticResource KeepLastNumber}`, which drops the null via `BindingOperations.DoNothing`.
- **`ScrollViewer.Padding` is excluded from the Extent (Avalonia 12).** At max scroll the content still sticks out below the viewport by the padding height, so the last line(s) are clipped and unreachable — even after `ScrollToEnd()`. Put the inset on the *content* (`Margin` on the inner `ItemsControl`/`StackPanel`) instead. Verified headlessly: 12,8,12,4 on the viewer → last item 20px below the viewport; same inset as content margin → fully visible.
- **`ScrollViewer.Padding` is excluded from the Extent (Avalonia 12).** At max scroll the content still sticks out below the viewport by the padding height, so the last line(s) are clipped and unreachable — even after `ScrollToEnd()`. Put the inset on the *content* (`Margin` on the inner `ItemsControl`/`StackPanel`) instead. Verified headlessly: 12,8,12,4 on the viewer → last item 20px below the viewport; same inset as content margin → fully visible. **`ListBox` does *not* have this trap** — its Fluent control-theme template forwards `Padding` onto `PART_ItemsPresenter`'s margin (content side), not the raw `ScrollViewer.Padding`; verified headlessly that `ListBox.Padding="10,4"` and a `Margin="10,4"` on the `ItemsPanelTemplate` panel produce an *identical* Extent and leave the last row fully visible at max scroll. Still fine to put the inset on the panel for consistency with the other scrollables above, just don't assume it's fixing a real bug there.
- **Never bind bare punctuation gestures.** Window key bindings live on `MainWindow` (`Ctrl+K` search, `Ctrl+N` add-task). `OemQuestion` once held search focus and silently swallowed `#` app-wide on a German layout.
- **`FocusClearing`'s Escape handler is scoped to `MainWindow`** (`AddClassHandler<MainWindow>`, not `<TopLevel>`) — it clears focus from a TextBox on Escape, mirroring click-outside. Modals are separate `Window` instances that bind their own Escape → close, so it never runs there. Mission Control's ConPTY tiles are in `MissionControlWindow`, also unaffected, so **Escape always reaches the PTY**.
- **Review gate:** Approve & Merge stays disabled until the diff has been opened once, and re-locks per run → [review-merge](../../docs/explore-notes/review-merge.md).
@@ -529,10 +529,11 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable
}
// Builds the hierarchy-aware flat ordering (top-level rows interleaved with visible children,
// orphans flagged so they render flat) and classifies it into Overdue/Open/Completed, exactly
// as Regroup renders it. Shared with callers that need "every completed row" independent of
// Rows/IsShowingCompleted (e.g. ClearCompletedAsync).
private (List<TaskRowViewModel> Overdue, List<TaskRowViewModel> Open, List<TaskRowViewModel> Completed) ClassifyItems()
// orphans flagged so they render flat) and partitions it into Overdue/Open/Completed. Pure
// with respect to chain state (ShowAsChainMember/ChainStep/ChainAfterLabel) — callers that
// only need section membership/counts (e.g. ClearCompletedAsync) should use this directly
// rather than ClassifyItems, which additionally mutates every row via ApplyChainGrouping.
private (List<TaskRowViewModel> Overdue, List<TaskRowViewModel> Open, List<TaskRowViewModel> Completed) PartitionItems()
{
// Items is already ordered by SortOrder from the DB query.
// Treat rows whose ParentTaskId is not in the current view as orphans -> top-level.
@@ -580,6 +581,17 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable
open.Add(r);
}
return (overdue, open, completed);
}
// ClassifyItems layers the chain-grouping mutation on top of PartitionItems, exactly as
// Regroup renders it — every row in the result has ShowAsChainMember/ChainStep/ChainAfterLabel
// written. Only call this from a path that actually renders the result; a caller that just
// needs section membership (e.g. a count) should call PartitionItems instead.
private (List<TaskRowViewModel> Overdue, List<TaskRowViewModel> Open, List<TaskRowViewModel> Completed) ClassifyItems()
{
var (overdue, open, completed) = PartitionItems();
// Dependency chains are resolved and pulled together per section (not on the
// pre-split `flat` list): a chain head might land in a different section than its
// dependent (e.g. a Done head in Completed, an open dependent in Open) — in that case
@@ -965,7 +977,7 @@ public sealed partial class TasksIslandViewModel : ViewModelBase, IDisposable
[RelayCommand]
private async Task ClearCompletedAsync()
{
var (_, _, completed) = ClassifyItems();
var (_, _, completed) = PartitionItems();
if (completed.Count == 0) return;
// Delete children before parents so the parent-child FK (Restrict) doesn't
@@ -54,8 +54,8 @@ public sealed partial class LogVisualizerViewModel : ViewModelBase
[RelayCommand]
public async Task RefreshAsync()
{
_all = await _worker.GetRecentLogsAsync();
Apply();
var latest = await _worker.GetRecentLogsAsync();
Reconcile(latest);
}
partial void OnWarnErrorOnlyChanged(bool value) => Apply();
@@ -68,10 +68,70 @@ public sealed partial class LogVisualizerViewModel : ViewModelBase
: _all;
foreach (var e in items.Reverse())
Rows.Add(new LogVisualizerRow(e.TimestampUtc.ToLocalTime().ToString("HH:mm:ss"), e.Message, e.Level));
UpdateStatusText();
}
// `_all` (oldest-first) mirrors the worker's LogRingBuffer: append-mostly, with the oldest
// entries falling off the front once they age past the 30-min window. A full Rows.Clear()
// on every 4s tick would reset the user's scroll position/selection while they're mid-read,
// so patch just the delta instead: entries still present keep their row, aged-out entries
// are trimmed off the (oldest) tail of Rows, new entries are inserted at the (newest) head.
private void Reconcile(IReadOnlyList<WorkerLogEntry> latest)
{
var previous = _all;
_all = latest;
if (previous.Count == 0)
{
Apply();
return;
}
var overlap = FindOverlap(previous, latest);
var evictedCount = previous.Count - overlap;
var added = latest.Skip(overlap).ToList();
if (evictedCount == 0 && added.Count == 0)
return; // nothing aged out, nothing new — leave Rows (and scroll/selection) untouched
if (evictedCount > 0)
{
var removeCount = WarnErrorOnly
? previous.Take(evictedCount).Count(e => e.Level is WorkerLogLevel.Warn or WorkerLogLevel.Error)
: evictedCount;
for (var i = 0; i < removeCount; i++)
Rows.RemoveAt(Rows.Count - 1);
}
foreach (var e in added)
{
if (WarnErrorOnly && e.Level is not (WorkerLogLevel.Warn or WorkerLogLevel.Error))
continue;
Rows.Insert(0, new LogVisualizerRow(e.TimestampUtc.ToLocalTime().ToString("HH:mm:ss"), e.Message, e.Level));
}
UpdateStatusText();
}
// The longest run of `previous`'s tail that reappears as `latest`'s head — i.e. how much of
// the old snapshot survived unevicted. 0 when nothing overlaps (e.g. right after opening).
private static int FindOverlap(IReadOnlyList<WorkerLogEntry> previous, IReadOnlyList<WorkerLogEntry> latest)
{
for (var k = Math.Min(previous.Count, latest.Count); k > 0; k--)
{
var matches = true;
for (var i = 0; i < k; i++)
{
if (!previous[previous.Count - k + i].Equals(latest[i])) { matches = false; break; }
}
if (matches) return k;
}
return 0;
}
private void UpdateStatusText() =>
StatusText = Rows.Count == 0
? Loc.T("modals.logVisualizer.empty")
: Loc.T("modals.logVisualizer.count", Rows.Count);
}
[RelayCommand]
private void Close()
@@ -91,15 +91,17 @@
<!-- Task list: one flat, virtualized ListBox. Rows is HeaderRow | TaskRowViewModel — group
headers are regular entries, not a separate ItemsControl per section, so a
VirtualizingStackPanel actually bounds the realized container count. -->
VirtualizingStackPanel actually bounds the realized container count.
Inset lives on the panel, NOT on the ListBox: Avalonia 12 leaves ScrollViewer.Padding
out of the Extent, so at max scroll the last row would sit below the viewport and stay
unreachable. -->
<ListBox x:Name="RowsListBox"
ItemsSource="{Binding Rows}"
Background="Transparent"
BorderThickness="0"
Padding="10,4">
BorderThickness="0">
<ListBox.ItemsPanel>
<ItemsPanelTemplate>
<VirtualizingStackPanel/>
<VirtualizingStackPanel Margin="10,4"/>
</ItemsPanelTemplate>
</ListBox.ItemsPanel>
<ListBox.Styles>
@@ -13,6 +13,14 @@ public class LogVisualizerViewModelTests
public override Task<IReadOnlyList<WorkerLogEntry>> GetRecentLogsAsync() => Task.FromResult(_logs);
}
// Simulates the worker's append-mostly LogRingBuffer across successive ticks: each
// RefreshAsync call returns whatever `Logs` currently holds.
private sealed class MutableFakeClient : StubWorkerClient
{
public IReadOnlyList<WorkerLogEntry> Logs { get; set; } = Array.Empty<WorkerLogEntry>();
public override Task<IReadOnlyList<WorkerLogEntry>> GetRecentLogsAsync() => Task.FromResult(Logs);
}
private static WorkerLogEntry E(WorkerLogLevel lvl, string msg)
=> new(msg, lvl, new DateTime(2026, 6, 23, 8, 0, 0, DateTimeKind.Utc));
@@ -176,4 +184,56 @@ public class LogVisualizerViewModelTests
Assert.False(string.IsNullOrEmpty(vm.StatusText));
}
[Fact]
public async Task Tick_with_no_new_data_does_not_touch_rows()
{
var t0 = new DateTime(2026, 6, 23, 8, 0, 0, DateTimeKind.Utc);
var client = new MutableFakeClient { Logs = new[] { E(WorkerLogLevel.Info, "a", t0) } };
var vm = new LogVisualizerViewModel(client);
await vm.RefreshAsync();
var row = vm.Rows.Single();
var changeCount = 0;
vm.Rows.CollectionChanged += (_, _) => changeCount++;
await vm.RefreshAsync(); // same snapshot on the next tick
Assert.Equal(0, changeCount);
Assert.Same(row, vm.Rows.Single());
}
[Fact]
public async Task Tick_appends_a_new_entry_without_clearing_existing_rows()
{
var t0 = new DateTime(2026, 6, 23, 8, 0, 0, DateTimeKind.Utc);
var client = new MutableFakeClient { Logs = new[] { E(WorkerLogLevel.Info, "a", t0) } };
var vm = new LogVisualizerViewModel(client);
await vm.RefreshAsync();
var existingRow = vm.Rows.Single();
client.Logs = new[] { E(WorkerLogLevel.Info, "a", t0), E(WorkerLogLevel.Info, "b", t0.AddMinutes(1)) };
await vm.RefreshAsync();
Assert.Equal(new[] { "b", "a" }, vm.Rows.Select(r => r.Message));
Assert.Same(existingRow, vm.Rows[1]); // the pre-existing row instance survived the tick
}
[Fact]
public async Task Tick_drops_the_row_for_an_entry_that_aged_out_of_the_window()
{
var t0 = new DateTime(2026, 6, 23, 8, 0, 0, DateTimeKind.Utc);
var client = new MutableFakeClient
{
Logs = new[] { E(WorkerLogLevel.Info, "a", t0), E(WorkerLogLevel.Info, "b", t0.AddMinutes(1)) },
};
var vm = new LogVisualizerViewModel(client);
await vm.RefreshAsync();
Assert.Equal(new[] { "b", "a" }, vm.Rows.Select(r => r.Message));
// "a" fell out of the ring buffer's 30-min window; "c" is a new entry.
client.Logs = new[] { E(WorkerLogLevel.Info, "b", t0.AddMinutes(1)), E(WorkerLogLevel.Info, "c", t0.AddMinutes(2)) };
await vm.RefreshAsync();
Assert.Equal(new[] { "c", "b" }, vm.Rows.Select(r => r.Message));
}
}
@@ -0,0 +1,156 @@
using ClaudeDo.Data;
using ClaudeDo.Data.Models;
using ClaudeDo.Ui.ViewModels.Islands;
using Microsoft.EntityFrameworkCore;
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
namespace ClaudeDo.Ui.Tests.ViewModels;
// ClassifyItems (used by Regroup) additionally mutates every row's chain-display fields via
// ApplyChainGrouping. ClearCompletedAsync only needs section membership/counts, so it goes
// through the pure PartitionItems instead — this must not write ShowAsChainMember/ChainStep/
// ChainAfterLabel as a side effect of counting completed rows.
public class TasksIslandClassifyItemsTests : IDisposable
{
private readonly string _dbPath;
public TasksIslandClassifyItemsTests()
{
_dbPath = Path.Combine(Path.GetTempPath(), $"claudedo_ui_classify_{Guid.NewGuid():N}.db");
using var ctx = NewContext();
ctx.Database.EnsureCreated();
}
public void Dispose()
{
try { File.Delete(_dbPath); } catch { }
try { File.Delete(_dbPath + "-wal"); } catch { }
try { File.Delete(_dbPath + "-shm"); } catch { }
}
private ClaudeDoDbContext NewContext()
{
var opts = new DbContextOptionsBuilder<ClaudeDoDbContext>()
.UseSqlite($"Data Source={_dbPath}")
.Options;
return new ClaudeDoDbContext(opts);
}
private sealed class TestDbFactory : IDbContextFactory<ClaudeDoDbContext>
{
private readonly Func<ClaudeDoDbContext> _create;
public TestDbFactory(Func<ClaudeDoDbContext> create) => _create = create;
public ClaudeDoDbContext CreateDbContext() => _create();
}
private TasksIslandViewModel BuildViewModel()
{
var factory = new TestDbFactory(NewContext);
return new TasksIslandViewModel(factory, worker: null);
}
private static ListNavItemViewModel UserList(string listEntityId, string name) =>
new() { Id = $"user:{listEntityId}", Kind = ListKind.User, Name = name };
private static async Task LoadAndWaitAsync(TasksIslandViewModel vm, ListNavItemViewModel list, int expectedCount = 1)
{
vm.LoadForList(list);
var deadline = DateTime.UtcNow.AddSeconds(5);
while (DateTime.UtcNow < deadline)
{
await Task.Delay(25);
if (vm.Items.Count >= expectedCount) break;
}
await Task.Delay(50);
}
private async Task SeedAsync(
params (string Id, TaskStatus Status, string? DependsOnTaskId, int SortOrder)[] tasks)
{
await using var db = NewContext();
db.Lists.Add(new ListEntity { Id = "list1", Name = "Default", CreatedAt = DateTime.UtcNow });
foreach (var t in tasks)
{
db.Tasks.Add(new TaskEntity
{
Number = TestTaskNumbers.Next(),
Id = t.Id,
ListId = "list1",
Title = t.Id,
CreatedAt = DateTime.UtcNow,
Status = t.Status,
DependsOnTaskId = t.DependsOnTaskId,
SortOrder = t.SortOrder,
});
}
await db.SaveChangesAsync();
}
[Fact]
public async Task ClearCompleted_counting_path_leaves_chain_fields_untouched()
{
await SeedAsync(
("head", TaskStatus.Idle, null, 0),
("dep", TaskStatus.Idle, "head", 1));
var vm = BuildViewModel();
await LoadAndWaitAsync(vm, UserList("list1", "Default"), expectedCount: 2);
var dep = vm.Items.Single(r => r.Id == "dep");
// Sanity: the initial Regroup (during load) computed the chain correctly.
Assert.True(dep.ShowAsChainMember);
Assert.Equal(1, dep.ChainStep);
// Corrupt the row to a sentinel distinguishable from both the correct and default value.
dep.ShowAsChainMember = false;
dep.ChainStep = 99;
dep.ChainAfterLabel = "SENTINEL";
// Nothing is completed, so ClearCompletedAsync does nothing but count via PartitionItems.
await vm.ClearCompletedCommand.ExecuteAsync(null);
Assert.False(dep.ShowAsChainMember);
Assert.Equal(99, dep.ChainStep);
Assert.Equal("SENTINEL", dep.ChainAfterLabel);
}
[Fact]
public async Task Regroup_still_recomputes_chain_fields()
{
await SeedAsync(
("head", TaskStatus.Idle, null, 0),
("dep", TaskStatus.Idle, "head", 1));
var vm = BuildViewModel();
await LoadAndWaitAsync(vm, UserList("list1", "Default"), expectedCount: 2);
var dep = vm.Items.Single(r => r.Id == "dep");
dep.ShowAsChainMember = false;
dep.ChainStep = 99;
dep.ChainAfterLabel = "SENTINEL";
vm.Regroup();
Assert.True(dep.ShowAsChainMember);
Assert.Equal(1, dep.ChainStep);
Assert.Null(dep.ChainAfterLabel);
}
[Fact]
public async Task ClearCompleted_still_deletes_completed_rows_and_reports_count()
{
await SeedAsync(
("open1", TaskStatus.Idle, null, 0),
("done1", TaskStatus.Done, null, 1),
("done2", TaskStatus.Done, null, 2));
var vm = BuildViewModel();
await LoadAndWaitAsync(vm, UserList("list1", "Default"), expectedCount: 3);
vm.IsShowingCompleted = true;
await vm.ClearCompletedCommand.ExecuteAsync(null);
Assert.DoesNotContain(vm.Items, r => r.Id is "done1" or "done2");
Assert.Contains(vm.Items, r => r.Id == "open1");
}
}