Settings polish: refine the Agent settings layout - #875
Conversation
Arrange the Agent page controls so agent selection, model configuration, pane behavior, error detection, sessions, and token usage follow the intended sequence without changing their content. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb1aab1-e7ab-4cb3-ab56-813c33c219c2
There was a problem hiding this comment.
🟡 Changes recommended
The PR metadata claims it closes #789 (hint text additions), but the diff only reorders UI elements and does not implement the issue’s requested hint/description text work.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR reorders the AI Agents settings page UI to present the core agent configuration options in a clearer sequence, while keeping existing bindings and strings intact.
Changes:
- Moves the “provider Yolo mode” setting block to appear after the requested settings sequence (ending with Token usage).
- Preserves existing bindings, UIDs, and related InfoBars while changing only the visual order.
File summaries
| File | Description |
|---|---|
| src/cascadia/TerminalSettingsEditor/AIAgents.xaml | Reorders the Yolo-mode setting block to match the intended Agent page settings sequence. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Use the same standard toggle layout as Sessions and Token usage so the On/Off label and switch align consistently on the right. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb1aab1-e7ab-4cb3-ab56-813c33c219c2
There was a problem hiding this comment.
🟡 Changes recommended
The diff introduces non-reorder UI/property changes (and the PR metadata claims to close #789 without adding hint text), so the stated “behavior unchanged” intent isn’t currently met.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/cascadia/TerminalSettingsEditor/AIAgents.xaml:438
- The PR description/metadata indicates this closes #789 (add hint/description text on settings pages), but this diff only reorders controls (and tweaks the Yolo toggle). Either remove the issue-closing linkage or include the intended hint-text additions (likely via SettingContainer
HelpText+ localized .resw updates).
<!-- Global Yolo preference: asks supported providers to enable
their advertised ACP session mode. High-risk toggle, so it
is disabled outright (IsYoloModePolicyLocked) when the
AllowYoloMode GPO policy blocks it. -->
<local:SettingContainer x:Name="AgentPaneYoloMode"
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
Place Add New in the same right-side action column as Remove and give both buttons the same minimum width. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb1aab1-e7ab-4cb3-ab56-813c33c219c2
There was a problem hiding this comment.
🟡 Changes recommended
The implemented layout does not fully deliver the stated “equal-width right-side column” alignment and the PR scope/“Closes #789” claim appears mismatched to the actual changes.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/cascadia/TerminalSettingsEditor/AIAgents.xaml:445
- PR metadata says this keeps existing copy/bindings/behavior unchanged and closes #789 (adding hint text), but this diff is strictly layout/styling reordering (and does not add hint/description text for settings). If this PR is intended to close #789, it likely needs additional help text updates; otherwise consider removing the issue closure reference to avoid prematurely closing the feature request.
<!-- Global Yolo preference: asks supported providers to enable
their advertised ACP session mode. High-risk toggle, so it
is disabled outright (IsYoloModePolicyLocked) when the
AllowYoloMode GPO policy blocks it. -->
<local:SettingContainer x:Name="AgentPaneYoloMode"
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
Use the same fixed width for Add New and Remove so localized labels cannot make their independent action columns diverge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb1aab1-e7ab-4cb3-ab56-813c33c219c2
There was a problem hiding this comment.
🟡 Changes recommended
Fixed-width localized buttons (Width="120") are likely to truncate longer translations (e.g. fr-FR/de-DE) and should be adjusted to avoid clipped UI labels.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/cascadia/TerminalSettingsEditor/AIAgents.xaml:316
Width="120"can clip localized text for this action button (e.g. de-DE is "Endpunkt hinzufügen", fr-FR is "Ajouter un point de terminaison"). UsingMinWidthkeeps the intended minimum column size but allows longer translations to render fully.
<Button Grid.Column="1"
x:Uid="AIAgents_CustomProviderAdd"
Click="{x:Bind ViewModel.AddCustomModelProvider}"
Width="120"
Style="{StaticResource AccentButtonStyle}" />
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
Use the standard control width for both custom model action buttons so they remain equal without truncating longer localized labels. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb1aab1-e7ab-4cb3-ab56-813c33c219c2
There was a problem hiding this comment.
🟢 Approval recommended
The changes are limited to XAML layout/sizing adjustments, use existing shared resources, and appear consistent with adjacent control patterns without introducing functional/binding changes.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
Keep Add New and Remove equal and right-aligned at the original compact 120-pixel width. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb1aab1-e7ab-4cb3-ab56-813c33c219c2
Rename the English Add New action to Add endpoint and keep localized resource comments aligned with the updated label. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: adb1aab1-e7ab-4cb3-ab56-813c33c219c2
There was a problem hiding this comment.
🔵 Needs a closer look
Two newly hard-coded 120px button widths risk truncating localized labels and should use the shared StandardBoxMinWidth sizing baseline to match existing settings UI conventions.
Review details
Suppressed comments (2)
src/cascadia/TerminalSettingsEditor/AIAgents.xaml:290
- The custom provider Remove button uses a hard-coded Width="120", which can truncate localized labels and diverges from the Settings Editor sizing baseline (StandardBoxMinWidth). Consider using the shared StandardBoxMinWidth width to keep alignment while reducing localization risk.
<Button Grid.Column="1"
x:Uid="AIAgents_CustomProviderRemove"
VerticalAlignment="Center"
Click="{x:Bind Remove}"
Width="120" />
src/cascadia/TerminalSettingsEditor/AIAgents.xaml:316
- The Add New custom provider button also uses Width="120". This can truncate localized strings and makes the action column narrower than the standard control sizing used elsewhere on settings pages; using StandardBoxMinWidth keeps the equal-width column intent while matching the shared sizing baseline.
<Button Grid.Column="1"
x:Uid="AIAgents_CustomProviderAdd"
Click="{x:Bind ViewModel.AddCustomModelProvider}"
Width="120"
Style="{StaticResource AccentButtonStyle}" />
- Files reviewed: 17/17 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The action buttons currently hard-code Width="120" in the changed XAML, which is likely to truncate localized labels and diverges from the established StandardBoxMinWidth sizing used elsewhere on the page.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
src/cascadia/TerminalSettingsEditor/AIAgents.xaml:290
- Using a fixed Width="120" for the localized Remove button risks truncating longer translations and is inconsistent with the established TerminalSettingsEditor sizing convention (StandardBoxMinWidth=248). To keep the right-side action column aligned without clipping, use StandardBoxMinWidth for the button width here.
This issue also appears on line 312 of the same file.
src/cascadia/TerminalSettingsEditor/AIAgents.xaml:316
- Using a fixed Width="120" for the localized Add endpoint button can truncate in some locales and diverges from the shared sizing used elsewhere on this page (StandardBoxMinWidth). Setting the width to StandardBoxMinWidth keeps the right column equal-width while allowing adequate space for translations.
<Button Grid.Column="1"
x:Uid="AIAgents_CustomProviderAdd"
Click="{x:Bind ViewModel.AddCustomModelProvider}"
Width="120"
Style="{StaticResource AccentButtonStyle}" />
- Files reviewed: 17/17 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
Testing
Closes #789