Hide UTC timestamps option when timestamps are disabled - #19931
Hide UTC timestamps option when timestamps are disabled#19931Shubhransh Gupta (shubhransh-gupta) wants to merge 2 commits into
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19931Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19931" |
There was a problem hiding this comment.
🟡 Changes recommended
The regression test has a missing namespace import and lacks required terminal JS interop setup.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Hides the UTC timestamp option when console timestamps are disabled.
Changes:
- Conditionally renders the UTC timestamp menu item.
- Adds focused bUnit regression coverage.
File summaries
| File | Description |
|---|---|
src/Aspire.Dashboard/Components/Pages/ConsoleLogs.razor.cs |
Gates the UTC option on timestamp visibility. |
tests/Aspire.Dashboard.Components.Tests/Pages/ConsoleLogsTerminalTests.cs |
Tests the hidden-option behavior. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
| resourceChannelProvider: () => resourceChannel, | ||
| initialResources: [terminalResource]); | ||
|
|
||
| SetupConsoleLogsServices(dashboardClient); |
There was a problem hiding this comment.
Addressed in commit f89a9a3: moved the test to ConsoleLogsTests.cs using a standard non-terminal resource (ModelTestHelpers.CreateResource), which is the appropriate home for non-terminal console menu tests and avoids needing terminal JS interop. Also expanded the test to verify both hidden and visible states when toggling timestamps.
| await cut.InvokeAsync(() => instance.HandleViewChangedForTestAsync(nameof(ConsoleLogs.ConsoleLogsView.Console))); | ||
| cut.WaitForState(() => instance.ActiveViewForTest == ConsoleLogs.ConsoleLogsView.Console); | ||
|
|
||
| var loc = Services.GetRequiredService<IStringLocalizer<Dashboard.Resources.ConsoleLogs>>(); |
There was a problem hiding this comment.
Addressed in commit f89a9a3: moved to ConsoleLogsTests.cs which already imports Microsoft.Extensions.Localization, and reverted ConsoleLogsTerminalTests.cs to unmodified.
|
@microsoft-github-policy-service agree |
Description
On the Console Logs page, the options menu previously showed the
UTC timestampsoption as a disabled checkbox even whenShow timestampswas turned off. This caused visual confusion (resembling a checkbox or broken glyph rather than an inactive setting).This PR conditionally renders the
UTC timestampsoption only when_showTimestampistrue. When timestamps are turned off, the option is cleanly hidden from the menu.Fixes #19019
Checklist