Skip to content

Visual feedback on Go button - bugs #95

Description

@JesseLiberty

Review from Copilot:

Copilot review overview
🟡 Changes recommended
Moderate correctness and test-determinism issues remain unresolved.

Findings: 3 Medium severity · 1 Low severity

In BlogWriter.Web.Tests/BlogWorkspaceServiceTests.cs:

  • public BlogWorkspaceServiceTests() =>
  •    // BlogWorkspaceService reports progress via IProgress<T>, which posts to
    
  •    // SynchronizationContext.Current. Without an ambient context (the default
    
  •    // under xUnit), Progress<T> falls back to ThreadPool.QueueUserWorkItem,
    
  •    // which races with the synchronous continuation after each awaited
    
  •    // operation and can append reviewer feedback out of order. Installing an
    
  •    // immediate, single-threaded context here makes that ordering
    
  •    // deterministic for every test in this class, matching how a Blazor
    
  •    // Server circuit's single-threaded dispatcher behaves in production.
    
  •    SynchronizationContext.SetSynchronizationContext(new ImmediateSynchronizationContext());
    
  • public void Dispose() => SynchronizationContext.SetSynchronizationContext(_previousSynchronizationContext);
  • private sealed class ImmediateSynchronizationContext : SynchronizationContext
  • {
  •    public override void Post(SendOrPostCallback d, object? state) => d(state);
    

Calling the callback inline does not make this context single-threaded: Post executes on whichever thread calls Report, so concurrent progress reports can still mutate the workspace concurrently. The helper therefore does not provide the deterministic ordering claimed in the constructor comment; use a serialized queue/dispatcher or a controlled test scheduler.

In BlogWriter.Web/wwwroot/app.css:

@@ -128,7 +128,9 @@ button, a { touch-action: manipulation; }
cursor: pointer;
}
.range-go:hover:not(:disabled) { background: var(--forest-dark); }
+.range-go:active:not(:disabled) { background: var(--forest-dark); }
This rule is not sufficient for the required drag-off behavior. :active is tied to the ongoing pointer press, not to the pointer remaining over the element, so a press dragged outside can keep the button dark until mouseup instead of returning to the idle color. Please use a modality-aware pointer-leave/cancel mechanism (while retaining keyboard feedback) rather than relying on :active alone.

This issue also appears on line 131 of the same file.

In BlogWriter.Web/wwwroot/app.css:

.range-go:disabled { border-color: var(--line); background: #ecece7; color: var(--muted); cursor: not-allowed; }
+.range-go:active:disabled { background: #ecece7; }
This disabled guard is unreachable for the current Go button: CommandBar.razor renders .range-go without a disabled attribute, and Home.razor passes only the Go callback. Consequently the disabled acceptance scenario in this feature cannot occur, and invalid-range/processing states can still press the button. Either bind the button's disabled state to the existing eligibility state or remove the disabled requirement from this feature.

In specs/010-go-button-press-colors/quickstart.md:

@@ -0,0 +1,59 @@
+# Quickstart: Validate Go Button Press Color Feedback

+## Prerequisites
+
+- .NET 10 SDK installed
+- Repository checked out on branch 010-go-button-press-colors
+- No additional packages or services required (pure front-end CSS change)
+
+## Setup
+
+```powershell
+cd E:\ai.net\blog\BlogWriter
This hard-coded local path makes the quickstart non-reproducible for other checkouts. Use a repository-root placeholder/instruction instead of E:\ai.net\blog\BlogWriter.

—

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions