Fix Plan Route side panel layout on iPad and iPhone landscape - #5698
Fix Plan Route side panel layout on iPad and iPhone landscape#5698aleksandr-tata wants to merge 8 commits into
Conversation
| crosshairCenterXConstraint?.constant = centerX | ||
| crosshairCenterYConstraint?.constant = centerY | ||
| let mapViewController = OARootViewController.instance().mapPanel.mapViewController | ||
| mapViewController.viewportXScale = Double(2 * centerX / targetScreenSize.width) |
There was a problem hiding this comment.
Do we need to update viewportXScale in the bottom-sheet layout? Previously Plan Route adjusted only the Y scale. In portrait centerX is always width / 2, so this forces viewportXScale to 1.0 and may change an existing horizontal viewport configuration unnecessarily. It seems this should only be adjusted for the side-panel layout
| } | ||
|
|
||
| override func hide(_ animated: Bool, duration: TimeInterval, onComplete: (() -> Void)?) { | ||
| restoreMapViewport() |
There was a problem hiding this comment.
restoreMapViewport() is called before the dismissal animation starts, so the map can jump back to its original viewport while the Plan Route panel is still visible and animating out. Could we restore the viewport in dismiss() after the animation completes, as before?
|
The grid already supported separate button positions for landscape, but it used the full map width and only accounted for overlays at the top and bottom. This PR introduces a new case: the persistent Plan Route panel on the left. Adding Overall, I don't see any critical issues with the grid code, but the following scenarios should be tested in the UI, as it is difficult to verify their behavior from the code alone:
The weather overlay and context menu transitions are especially important because |
alex-dev-neo
left a comment
There was a problem hiding this comment.
Code review of the Plan Route side-panel layout. Three correctness findings below (two confirmed, one plausible).
Things that checked out clean: RTL handling (cross-panel constraints correctly use physical left/right, in-panel content uses leading/trailing); moving resetToDefaultRulerLayout out of the needUpdate block (no recursion — both setExternalLeftOverlay and setExternalRulerLeftOffset guard on value change); the reordering in restoreMapViewport (restoring the viewport scale before reanchorMapTarget is the correct order); and mapToolbar minimum content width on the smallest supported iPhone landscape (~301pt available vs ~244pt required).
| return layoutGeometryChanged | ||
| } | ||
|
|
||
| private func shouldUseSidePanelLayout(for size: CGSize) -> Bool { |
There was a problem hiding this comment.
The fixed 409pt panel is forced on every iPad regardless of window width.
shouldUseSidePanelLayout switches on the device idiom (OAUtilities.isIPad()) rather than on the width that is actually available. Resources/OsmAnd-Info.plist sets UIRequiresFullScreen=false, so on iPad the app runs in Split View / Slide Over / Stage Manager at roughly 320–375pt wide. sidePanelMapInset is then 409 (16 + 393), wider than the whole window:
topToolbar's side-panel constraints —left == sheetView.rightAnchor(409) together withright == view.safeAreaLayoutGuide.rightAnchor(~320) — require a negative width, so Auto Layout breaks one of them and the Close / Save / Options row ends up off-screen. Plan Route can no longer be exited.mapToolbarhas the identicalleft/rightpair and collapses the same way.mapCenterX(for:)returns ~window width, soviewportXScaleis set to ~2.0 and the crosshair is pinned to the right screen edge with no visible map underneath it.
OAUtilities.isWindowed already exists for exactly this case (it is used in additionalLandscapeOffset) and isn't consulted here. Gating on the actual size.width (e.g. requiring at least sidePanelMapInset + some minimum map width) would cover Split View, Slide Over and Stage Manager in one go.
| } | ||
|
|
||
| @discardableResult | ||
| private func applyLayoutMode(for size: CGSize) -> Bool { |
There was a problem hiding this comment.
sheetState is never normalized when entering side-panel layout, which can leave the map HUD permanently hidden.
applyLayoutMode swaps every constraint and flag but leaves sheetState untouched, and the overridden currentState (line 383) keeps returning it.
Repro: on iPhone in portrait, drag the sheet up to .fullScreen (map HUD controls are correctly hidden at that point), then rotate to landscape. viewWillTransition / applyLayoutMode set usesSidePanelLayout = true, but sheetState stays .fullScreen. OAMapHudViewController.updateTopControlsVisibility and updateBottomControlsVisibility both compute
isPlanRouteFullscreen = activeTargetType == OATargetRoutePlanning
&& scrollableHudViewController
&& scrollableHudViewController.currentState == EOADraggableMenuStateFullScreen;so they keep the zoom / my-location / 3D / compass controls hidden for the whole landscape session — precisely the controls this PR repositions next to the panel.
There is no way out of the state while the side panel is up: sheetPanRecognizer is disabled, topPartView.onTap early-returns on usesSidePanelLayout, and onSegmentChanged / onSegmentTapped only act when sheetState == .initial. Resetting sheetState to .expanded (or having currentState report a non-fullscreen state while usesSidePanelLayout) would fix it.
| if usesSidePanelLayout { | ||
| let minX = min(bounds.maxX, bounds.minX + sidePanelMapInset) | ||
| let maxX = max(minX, bounds.maxX - view.safeAreaInsets.right) | ||
| return CGRect(x: minX, y: bounds.minY, width: maxX - minX, height: bounds.height) |
There was a problem hiding this comment.
The side-panel branch returns the full view height, so “fit track” runs under both toolbars.
The bottom-sheet branch just below (lines 108–111) subtracts getNavbarHeight() from minY; this branch uses height: bounds.height and also doesn't account for the newly added bottom map toolbar.
PlanRouteAnalyzeViewController.refreshChartOnMap assigns mapViewportBounds to TrackChartHelper.screenBBox and calls refreshChart(fitTrack: true). On iPad (and iPhone landscape) the box therefore claims the 70pt top toolbar band (Close / title / Options / Save) and the ~68pt bottom map toolbar band as map surface, so a fitted track is centred too tall and its top and bottom end up beneath those controls.
Subtracting getNavbarHeight() from the top and sidePanelMapControlsReservedHeight(for:) from the bottom would mirror what the bottom-sheet branch already does.
Summary
Implemented the Plan Route side-panel layout for:
The existing bottom-sheet layout remains unchanged on iPhone in portrait.
Changes