fix(ui): surface precondition hints for silently-disabled review buttons (UX-Audit #2)

Cancel now shows an amber hint + tooltip while a merge drain is in
progress (ShowMergeDrainHint mirrors IsMergeDraining). Continue,
Reset & Retry, Open Diff, Worktree, and Review Combined Diff gain
ToolTip.Tip + ToolTip.ShowOnDisabled explaining why they're locked.
No CanExecute or behavior changed, only added communication.
This commit is contained in:
mika kuns
2026-08-21 09:31:56 +02:00
parent 4fc64732ec
commit dbb73bae38
5 changed files with 60 additions and 5 deletions
+6 -1
View File
@@ -253,7 +253,9 @@
} }
}, },
"agent": { "agent": {
"openWorktreeTip": "Worktree im Datei-Explorer öffnen" "openWorktreeTip": "Worktree im Datei-Explorer öffnen — benötigt einen aktiven Worktree",
"openDiffTip": "Diff öffnen — benötigt einen Worktree oder einen gemergten Commit-Bereich",
"reviewCombinedDiffTip": "Nur für Planning-Tasks mit Subtasks oder Child-Outcomes verfügbar"
}, },
"notes": { "notes": {
"today": "Heute", "today": "Heute",
@@ -268,6 +270,9 @@
"chipFailed": "FEHLGESCHLAGEN", "chipFailed": "FEHLGESCHLAGEN",
"reviewContinueTip": "Dieses Feedback senden und die Aufgabe erneut ausführen", "reviewContinueTip": "Dieses Feedback senden und die Aufgabe erneut ausführen",
"reviewResetTip": "Alle Änderungen verwerfen und die Aufgabe auf Leerlauf zurücksetzen", "reviewResetTip": "Alle Änderungen verwerfen und die Aufgabe auf Leerlauf zurücksetzen",
"mergeDrainHint": "Merge zieht Subtasks ein — Abbrechen entsperrt sich danach",
"continueNoSessionTip": "Setzt den letzten Lauf fort — benötigt eine vorherige Session",
"resetAndRetryInteractiveTip": "Interaktive Sitzung ist offen — erst schließen",
"reviewFeedbackPlaceholder": "Feedback für nächsten Lauf" "reviewFeedbackPlaceholder": "Feedback für nächsten Lauf"
}, },
"missionControl": { "missionControl": {
+7 -2
View File
@@ -253,7 +253,9 @@
} }
}, },
"agent": { "agent": {
"openWorktreeTip": "Open worktree in file explorer" "openWorktreeTip": "Open worktree in file explorer — requires an active worktree",
"openDiffTip": "Open the diff — requires a worktree or a merged commit range",
"reviewCombinedDiffTip": "Only available for planning tasks with subtasks or child outcomes"
}, },
"notes": { "notes": {
"today": "Today", "today": "Today",
@@ -268,7 +270,10 @@
"chipFailed": "FAILED", "chipFailed": "FAILED",
"reviewContinueTip": "Send this feedback and re-run the task", "reviewContinueTip": "Send this feedback and re-run the task",
"reviewResetTip": "Discard all changes and reset the task to Idle", "reviewResetTip": "Discard all changes and reset the task to Idle",
"reviewFeedbackPlaceholder": "Feedback for next run" "reviewFeedbackPlaceholder": "Feedback for next run",
"mergeDrainHint": "Merge is draining subtasks — Cancel unlocks once it finishes",
"continueNoSessionTip": "Resumes the last run — needs a previous session to continue",
"resetAndRetryInteractiveTip": "Interactive session is open — close it first"
}, },
"missionControl": { "missionControl": {
"windowTitle": "Mission Control", "windowTitle": "Mission Control",
@@ -1364,8 +1364,13 @@ public sealed partial class DetailsIslandViewModel : ViewModelBase, IDisposable
// fix); this just keeps the button from inviting a click the worker will reject anyway. // fix); this just keeps the button from inviting a click the worker will reject anyway.
[ObservableProperty] [ObservableProperty]
[NotifyCanExecuteChangedFor(nameof(CancelReviewCommand))] [NotifyCanExecuteChangedFor(nameof(CancelReviewCommand))]
[NotifyPropertyChangedFor(nameof(ShowMergeDrainHint))]
private bool _isMergeDraining; private bool _isMergeDraining;
// Drives the visible hint next to Cancel while a merge drain is in progress, so the
// silently-disabled button doesn't look broken.
public bool ShowMergeDrainHint => IsMergeDraining;
[RelayCommand(CanExecute = nameof(CanCancelReview))] [RelayCommand(CanExecute = nameof(CanCancelReview))]
private async System.Threading.Tasks.Task CancelReviewAsync() private async System.Threading.Tasks.Task CancelReviewAsync()
{ {
@@ -174,10 +174,14 @@
<StackPanel Orientation="Horizontal" Spacing="8"> <StackPanel Orientation="Horizontal" Spacing="8">
<Button Classes="btn accent" Content="Continue" <Button Classes="btn accent" Content="Continue"
Command="{Binding ContinueCommand}" Command="{Binding ContinueCommand}"
IsVisible="{Binding ShowContinue}" /> IsVisible="{Binding ShowContinue}"
ToolTip.Tip="{loc:Tr session.continueNoSessionTip}"
ToolTip.ShowOnDisabled="True" />
<Button Classes="btn" Content="Reset &amp; Retry" <Button Classes="btn" Content="Reset &amp; Retry"
Command="{Binding ResetAndRetryCommand}" Command="{Binding ResetAndRetryCommand}"
IsVisible="{Binding ShowResetAndRetry}" /> IsVisible="{Binding ShowResetAndRetry}"
ToolTip.Tip="{loc:Tr session.resetAndRetryInteractiveTip}"
ToolTip.ShowOnDisabled="True" />
</StackPanel> </StackPanel>
</StackPanel> </StackPanel>
</Border> </Border>
@@ -341,9 +345,12 @@
<!-- Inspect: diff / worktree / combined diff --> <!-- Inspect: diff / worktree / combined diff -->
<WrapPanel Orientation="Horizontal"> <WrapPanel Orientation="Horizontal">
<Button Classes="btn" Content="Open Diff" Margin="0,0,8,8" <Button Classes="btn" Content="Open Diff" Margin="0,0,8,8"
ToolTip.Tip="{loc:Tr agent.openDiffTip}"
ToolTip.ShowOnDisabled="True"
Command="{Binding Merge.OpenDiffCommand}" /> Command="{Binding Merge.OpenDiffCommand}" />
<Button Classes="btn" Margin="0,0,8,8" <Button Classes="btn" Margin="0,0,8,8"
ToolTip.Tip="{loc:Tr agent.openWorktreeTip}" ToolTip.Tip="{loc:Tr agent.openWorktreeTip}"
ToolTip.ShowOnDisabled="True"
Command="{Binding Merge.OpenWorktreeCommand}"> Command="{Binding Merge.OpenWorktreeCommand}">
<StackPanel Orientation="Horizontal" Spacing="5"> <StackPanel Orientation="Horizontal" Spacing="5">
<TextBlock Text="Worktree" /> <TextBlock Text="Worktree" />
@@ -351,6 +358,8 @@
</StackPanel> </StackPanel>
</Button> </Button>
<Button Classes="btn" Content="Review Combined Diff" Margin="0,0,8,8" <Button Classes="btn" Content="Review Combined Diff" Margin="0,0,8,8"
ToolTip.Tip="{loc:Tr agent.reviewCombinedDiffTip}"
ToolTip.ShowOnDisabled="True"
Command="{Binding Merge.ReviewCombinedDiffCommand}" /> Command="{Binding Merge.ReviewCombinedDiffCommand}" />
</WrapPanel> </WrapPanel>
</StackPanel> </StackPanel>
@@ -371,6 +380,17 @@
Text="Open the diff to enable merge" /> Text="Open the diff to enable merge" />
</StackPanel> </StackPanel>
<!-- Gate: a unit merge is draining this task's subtasks, so Cancel is locked
until it finishes (mirrors the worker-side guard). -->
<StackPanel Orientation="Horizontal" Spacing="6"
IsVisible="{Binding ShowMergeDrainHint}">
<PathIcon Data="{StaticResource Icon.Warning}" Width="11" Height="11"
Foreground="{DynamicResource AmberBrush}" VerticalAlignment="Center" />
<TextBlock Classes="meta" VerticalAlignment="Center"
Foreground="{DynamicResource AmberBrush}"
Text="{loc:Tr session.mergeDrainHint}" />
</StackPanel>
<WrapPanel Orientation="Horizontal"> <WrapPanel Orientation="Horizontal">
<StackPanel Orientation="Horizontal" Spacing="{StaticResource SpaceSm}" Margin="0,0,8,8"> <StackPanel Orientation="Horizontal" Spacing="{StaticResource SpaceSm}" Margin="0,0,8,8">
<Button Classes="btn accent" Content="Approve &amp; Merge" <Button Classes="btn accent" Content="Approve &amp; Merge"
@@ -384,6 +404,8 @@
<controls:OperationIndicator Status="{Binding ParkOp}" /> <controls:OperationIndicator Status="{Binding ParkOp}" />
</StackPanel> </StackPanel>
<Button Classes="btn" Content="Cancel" Margin="0,0,8,8" <Button Classes="btn" Content="Cancel" Margin="0,0,8,8"
ToolTip.Tip="{loc:Tr session.mergeDrainHint}"
ToolTip.ShowOnDisabled="True"
Command="{Binding CancelReviewCommand}" /> Command="{Binding CancelReviewCommand}" />
</WrapPanel> </WrapPanel>
@@ -138,6 +138,24 @@ public class DetailsIslandReviewActionsTests : IDisposable
Assert.False(vm.ApproveReviewCommand.CanExecute(null)); Assert.False(vm.ApproveReviewCommand.CanExecute(null));
} }
[Fact]
public void ShowMergeDrainHint_IsTrueExactlyWhenMergeIsDraining()
{
var vm = BuildVm(new RecordingWorkerClient());
vm.Bind(new TaskRowViewModel { Id = "task-drain-1", Status = TaskStatus.WaitingForReview });
Assert.False(vm.ShowMergeDrainHint);
Assert.True(vm.CancelReviewCommand.CanExecute(null));
vm.IsMergeDraining = true;
Assert.True(vm.ShowMergeDrainHint);
Assert.False(vm.CancelReviewCommand.CanExecute(null));
vm.IsMergeDraining = false;
Assert.False(vm.ShowMergeDrainHint);
Assert.True(vm.CancelReviewCommand.CanExecute(null));
}
private sealed class ThrowingWorkerClient : StubWorkerClient private sealed class ThrowingWorkerClient : StubWorkerClient
{ {
public override bool IsConnected => true; public override bool IsConnected => true;