[ios] non-notched layout #258
Merged
Conversation
This file contains 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
michaelkirk
commented
Sep 19, 2024
| // // Overlay safeArea | ||
| // Rectangle().fill(.blue.opacity(0.5)).padding(overlaySafeAreaInsets) | ||
| // } | ||
| // ) |
There was a problem hiding this comment.
I've since removed this commented out debug code, but it might help you review to see what it looked like:
The red part is the MapView, the yellow+red=orange part represents the safeAreaInsets added by this PR, and then the blue+yellow+red=grey middle part is where the Overlay's content goes.
| Device Type | Screenshot 1 | Screenshot 2 |
|---|---|---|
| Notched Device |  |
 |
| Non-Notched Device |  |
 |
michaelkirk
commented
Sep 19, 2024
apple/Sources/FerrostarMapLibreUI/Models/NavigationMapViewContentInsetMode.swift
Outdated
Show resolved
Hide resolved
michaelkirk
force-pushed
the
mkirk/non-notched-layout
branch
from
60a251a to
3899276
Compare
michaelkirk
commented
Sep 19, 2024
apple/Sources/FerrostarMapLibreUI/Views/Overylays/LandscapeNavigationOverlayView.swift
Show resolved
Hide resolved
michaelkirk
commented
Sep 19, 2024
michaelkirk
force-pushed
the
mkirk/non-notched-layout
branch
from
3899276 to
58d4afc
Compare
michaelkirk
commented
Sep 19, 2024
|
Wow 😍 It might take me till early next week to get to this, but this is awesome! Thanks for all the comparison screenshots and thorough comments! |
michaelkirk
commented
Sep 19, 2024
michaelkirk
commented
Sep 19, 2024
...FerrostarSwiftUITests/Views/__Snapshots__/ArrivalViewTests/testArrivalViewDefaultTheme.1.png
Outdated
Show resolved
Hide resolved
michaelkirk
force-pushed
the
mkirk/non-notched-layout
branch
2 times, most recently
from
4c35a29 to
8b22fd7
Compare
This will not compound with safeAreaInsets, but ensure that devices which don't have safeAreaInsets (like the iPhoneSE 3) will have some padding between the overlay and the screen edge. Now that the padding is applied to the Overlay view, some of the padding previously added to views within the overlay view is now redundant. remove double padding which is now redundant with safeAreaInsets
I think this is an improvement on all platforms, because it shows a tiny bit more of the underlying map, while still being _enough_ padding, but it's especially important for SE (non-notched devices), which is otherwise very pressed for space near the puck.
…ton becomes two lines which looks bad
e.g. the iPhone SE 2 and 3
The view already has the methods necessary to conform to the protocol, and both of the similar PortraitNavigationView and DynamicallyOrientingNavigationView have this conformance, so presumably this was just an accidental omission.
These changes are expected - due to shrinking the height of the arrival view a bit.
michaelkirk
force-pushed
the
mkirk/non-notched-layout
branch
from
8b22fd7 to
9034185
Compare
Archdoog
reviewed
Sep 21, 2024
apple/Sources/FerrostarMapLibreUI/Models/NavigationMapViewContentInsetMode.swift
Outdated
Show resolved
Hide resolved
apple/Sources/FerrostarMapLibreUI/Models/NavigationMapViewContentInsetMode.swift
Outdated
Show resolved
Hide resolved
apple/Sources/FerrostarMapLibreUI/Models/NavigationMapViewContentInsetMode.swift
Outdated
Show resolved
Hide resolved
Better names and docs
ianthetechie
approved these changes
Sep 24, 2024
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.
FIXES #242
This branch includes several commits. I can break them into separate PR's if you'd prefer, but they are intended to be both well factored (one issue addressed per commit) and related (layout is a bit whack-a-mole, so merging them independently might make some things worse before everything is better).