Skip to content

Fix Plan Route side panel layout on iPad and iPhone landscape - #5698

Open
aleksandr-tata wants to merge 8 commits into
masterfrom
task_5668_landscape_ipad_layout
Open

Fix Plan Route side panel layout on iPad and iPhone landscape#5698
aleksandr-tata wants to merge 8 commits into
masterfrom
task_5668_landscape_ipad_layout

Conversation

@aleksandr-tata

@aleksandr-tata aleksandr-tata commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Implemented the Plan Route side-panel layout for:

  • iPad in portrait and landscape
  • iPhone in landscape
  • macOS

The existing bottom-sheet layout remains unchanged on iPhone in portrait.

Changes

  • Added a fixed-width 393 pt side panel on the left side of the screen.
  • Added RTL support while keeping the side panel physically aligned to the left.
  • Hidden the sheet grabber and disabled dragging in side-panel mode.
  • Moved the POI, Undo, Redo, and Route actions from the sheet to the map.
  • Preserved the RTL action order used by the portrait bottom sheet.
  • Applied the standard map-button styling to the relocated actions.
  • Removed the top navigation-bar gradient in side-panel mode.
  • Repositioned the map HUD controls outside the side panel.
  • Kept the Route Between Points and Zoom/Location controls in their physical map positions in RTL layouts.
  • Aligned the bottom action row with the visible map area and device safe areas.
  • Positioned the 3D, Location, Zoom In, and Zoom Out controls above the bottom action row.
  • Recalculated the map center, crosshair, and viewport using the actual visible map area.
  • Added safe-area handling for the initial presentation and device rotation.
  • Preserved the map target and horizontal and vertical viewport scales while updating the layout.
  • Restored the original map viewport before dismissing Plan Route.
  • Kept the map interactive through empty areas of the map action toolbar.

@aleksandr-tata
aleksandr-tata requested a review from tigrim August 26, 2026 12:29
@aleksandr-tata aleksandr-tata linked an issue Aug 26, 2026 that may be closed by this pull request
6 tasks
@aleksandr-tata
aleksandr-tata marked this pull request as draft August 26, 2026 12:35
@aleksandr-tata
aleksandr-tata marked this pull request as ready for review August 26, 2026 20:37
crosshairCenterXConstraint?.constant = centerX
crosshairCenterYConstraint?.constant = centerY
let mapViewController = OARootViewController.instance().mapPanel.mapViewController
mapViewController.viewportXScale = Double(2 * centerX / targetScreenSize.width)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@DmitrySvetlichny

DmitrySvetlichny commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

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 externalLeftOverlayPx looks reasonable and follows the existing approach used by externalTopOverlay and externalBottomOverlay.

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:

  • Switch between portrait and landscape while the Plan Route panel is fully expanded/fullscreen and verify that all map buttons remain visible and correctly positioned.
  • Enable and disable the weather overlay and verify that the map buttons do not overlap or move incorrectly.
  • Open and close different map context menus and verify that the map buttons remain correctly positioned.

The weather overlay and context menu transitions are especially important because externalTopOverlay and externalBottomOverlay were originally introduced to prevent map buttons from breaking in these cases.

@alex-dev-neo alex-dev-neo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 with right == 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.
  • mapToolbar has the identical left/right pair and collapses the same way.
  • mapCenterX(for:) returns ~window width, so viewportXScale is 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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Landscape / iPad layout for Plan a Route

4 participants