Logs tab, Responsive Layouts - #4
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR moves container log viewing into the main window (as a “Logs” tab) and introduces responsive UI behaviors for small screens and compact card layouts, alongside related DI/service cleanup and new tests.
Changes:
- Replaces the dedicated
LogsWindowwith an in-appLogsViewand adds “LOGS” navigation (including a small-screen hamburger menu). - Adds responsive state (
IsSmallScreen,IsCompactMode) to drive compact layouts for container/image cards. - Refactors log viewing into new/updated view models (
LogsViewModel,LogsPanelViewModel) and removes dialog-based log window plumbing.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 31 comments.
Show a summary per file
| File | Description |
|---|---|
| Views/MainWindow.axaml.cs | Tracks window width changes to toggle IsSmallScreen. |
| Views/MainWindow.axaml | Adds responsive navigation + embeds LogsView in main content. |
| Views/LogsWindow.axaml(.cs) | Removes the standalone logs window implementation. |
| Views/Components/LogsView.axaml(.cs) | Adds the new logs tab UI (container selector, search, error overview). |
| Views/Components/LogsPanel.axaml(.cs) | Introduces a log panel component/view model (currently appears unused). |
| Views/Components/ImageCard.axaml(.cs) | Adds compact-mode UI (buttons collapse into a menu). |
| Views/Components/ContainerCard.axaml(.cs) | Adds compact-mode UI + menu actions + stats toggle wiring. |
| ViewModels/MainWindowViewModel.cs | Adds ShowLogs, IsSmallScreen, logs navigation, and log-loading flow. |
| ViewModels/LogsViewModel.cs | Refactors logs to support selection, filtering, “search all”, and error overview. |
| ViewModels/LogsPanelViewModel.cs | New view model for panel-style log streaming/export/close. |
| ViewModels/ImageViewModel.cs | Adds IsCompactMode. |
| ViewModels/ContainerViewModel.cs | Adds IsCompactMode. |
| Services/ServiceCollectionExtensions.cs | Removes the LogsViewModel factory registration. |
| Services/IDialogService.cs / Services/DialogService.cs | Removes log window API and implementation. |
| OrbitalDocking.Tests/ViewModels/ResponsiveLayoutTests.cs | Adds unit tests for compact/small-screen breakpoints. |
| OrbitalDocking.Tests/ViewModels/LogsViewModelTests.cs | Adds unit tests around filtering/error overview/commands (currently with issues). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+55
to
+56
| if (value is not null) | ||
| _ = LoadContainerLogsAsync(value.Id, value.Name); |
There was a problem hiding this comment.
SelectedContainer change triggers LoadContainerLogsAsync (fire-and-forget), but callers (e.g. ShowContainerLogs, ViewContainerLogs) also call LoadContainerLogsAsync explicitly. This causes duplicate loads/stream restarts and can cancel an in-flight stream unexpectedly. Consider choosing one mechanism: either remove the OnSelectedContainerChanged auto-load, or remove the explicit LoadContainerLogsAsync calls and await/serialize the single load task.
Suggested change
| if (value is not null) | |
| _ = LoadContainerLogsAsync(value.Id, value.Name); | |
| // Intentionally left blank: logs are loaded explicitly via commands | |
| // to avoid duplicate loads and stream restarts. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Started with thinking logs needed to not be a second window, ended up fixing some reactivity stuff that was bugging me.
robot spam follows:
Testing improvements
LogsViewModelTests.cs, ensuring correct behavior for log loading, searching, error handling, and safe disposal.ResponsiveLayoutTests.csto verify compact mode toggling and screen breakpoints for container and image cards.Log viewing refactor
LogsPanelViewModelclass to encapsulate log streaming, exporting, and UI commands, separating concerns and improving code clarity.LogsViewModelto support multiple containers, error overview, search filtering, and improved log streaming lifecycle management. [1] [2] [3]Dependency injection and service changes
DialogService, simplifying dependency injection and service registration. [1] [2]IDialogServiceinterface to remove theShowLogsWindowmethod, reflecting the new approach for log viewing.UI properties
IsCompactModeobservable property toContainerViewModelandImageViewModelto support responsive layouts and compact mode toggling. [1] [2]