fix(mobile): width and swipe - #80
Merged
Merged
Conversation
… the hierarchy - Insets move off ScrollViewer.Padding onto the content (settings panes, camera page Majestic/ISP/metrics, player event list): the padding shrank the viewport but not the content, so it hung past the right edge on a phone - Health Center sheet drops its 420px minimum, wider than most phones - Android Back / edge swipe (and the mouse back button) closes the top sheet, leaves fullscreen, returns from a camera or recording, a settings section to its list, then to the start page; only at the root does the app go to background - A sheet closed by Back completes like Cancel, so the awaiting command and the bottom nav are not left disabled
- IBackNavigable: a page or overlay dialog handles Back first and only leaves or closes once it is at its own top - file manager climbs to the parent folder, closing at "/" - settings section returns to the list; an expanded month calendar folds back to its week strip before Back leaves the recordings page
PR Summary by QodoFix mobile page overflow and make Back unwind navigation levels
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
Code Review by Qodo
1. Discovery continues after its sheet closes
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+75
to
+76
| if (inner?.TryGoBack() != true) | ||
| top.Dismissed.TrySetResult(); |
There was a problem hiding this comment.
1. Discovery continues after its sheet closes 🐞 Bug ☼ Reliability
TryDismissTopmost completes only the presenter’s dismissal task, bypassing the dialog content’s Finish method and its cancellation call. When Back closes an active discovery sheet, DiscoveryDialogViewModel.Cancel() is never called, so its scan and fingerprint work can continue after the sheet disappears.
Agent Prompt
## Issue description
Back removes mobile dialogs without invoking their cancellation paths, leaving background work active.
## Fix Focus Areas
- src/OpenIPC.Viewer.App/Services/OverlayDialogPresenter.cs[69-80]
- src/OpenIPC.Viewer.App/Views/Dialogs/DiscoveryDialogContent.axaml.cs[43-49]
- src/OpenIPC.Viewer.App/Views/Dialogs/CameraEditorContent.axaml.cs[75-80]
## Recommended Fix
Give the presenter a way to invoke each content’s normal cancel operation before dismissing it. Ensure dismissal still completes the awaiting command and runs cleanup exactly once.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+99
to
+102
| if ((RemotePath ?? "/").Trim('/').Length == 0) | ||
| return false; | ||
| _ = NavigateUpAsync(); | ||
| return true; |
There was a problem hiding this comment.
2. Folder back can mix directory listings 🐞 Bug ≡ Correctness
FileManagerViewModel.TryGoBack starts NavigateUpAsync without awaiting or serializing the resulting directory load. Repeated Back presses, or Back during an existing load, let asynchronous enumerations clear and append to the same Entries collection after RemotePath has changed.
Agent Prompt
## Issue description
Back can overlap file-manager directory loads, allowing results from different paths into one listing.
## Fix Focus Areas
- src/OpenIPC.Viewer.App/ViewModels/FileManagerViewModel.cs[96-103]
- src/OpenIPC.Viewer.App/ViewModels/FileManagerViewModel.cs[219-239]
## Recommended Fix
Cancel or version outstanding loads when navigating, and apply entries and busy state only for the current path. Prevent an older enumeration from appending results after a newer navigation starts.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+221
to
+223
| case RecordingPlayerPageViewModel: | ||
| Receive(new GoBackToRecordingsMessage()); | ||
| return true; |
There was a problem hiding this comment.
3. Back from an event opens the wrong page 🐞 Bug ≡ Correctness
MainWindowViewModel.TryGoBack sends GoBackToRecordingsMessage for every recording player instead of using the page that opened it. A recording opened from Events therefore returns to Recordings on system Back, while recordings opened from the Recordings page take the same route.
Agent Prompt
## Issue description
System Back from a recording opened in Events navigates to Recordings instead of its origin.
## Fix Focus Areas
- src/OpenIPC.Viewer.App/ViewModels/MainWindowViewModel.cs[219-225]
- src/OpenIPC.Viewer.App/ViewModels/MainWindowViewModel.cs[304-326]
## Recommended Fix
Store the originating page when opening a recording from a non-player page, preserve it across previous/next recording changes, and return to it on system Back after disposing the player.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| Foreground="{StaticResource TextPrimaryBrush}"> | ||
|
|
||
| <Grid RowDefinitions="Auto,Auto,*,Auto" Margin="24" MinWidth="420"> | ||
| <Grid RowDefinitions="Auto,Auto,*,Auto" Margin="24" MinWidth="300"> |
There was a problem hiding this comment.
4. Health center still overflows narrow phones 🐞 Bug ≡ Correctness
HealthCenterContent replaces its grid’s 420-pixel minimum width with a fixed 300-pixel minimum while retaining a 24-pixel margin on each side. On a 320-pixel-wide viewport, the grid requires at least 348 pixels, so the sheet still cannot fit its content within the screen.
Agent Prompt
## Issue description
The Health Center grid’s minimum width plus its margins still exceeds a narrow mobile viewport.
## Fix Focus Areas
- src/OpenIPC.Viewer.App/Views/Dialogs/HealthCenterContent.axaml[14-14]
## Recommended Fix
Remove the fixed minimum width or make the grid and its margins responsive to available width. Verify that the sheet fits a 320-pixel-wide viewport.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
1 participant
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.
Summary
Related
Type
Checklist
TreatWarningsAsErrors=true).dotnet test); new Core logic has unit tests.AppreferencesCoreonly (Infrastructure / Video / Devices wired via DI in a head).Platforms tested
Screenshots / notes