Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added safe-area handling to BottomBannerViewController. #1982

Merged
merged 10 commits into from Mar 5, 2019

Conversation

JThramer
Copy link
Contributor

@JThramer JThramer commented Feb 19, 2019

It's the safety dance!

Fixes #1978. Adds a "padding" view that accounts for the device's bottom safe area, if any.

Test reference:
testbottombannerviewcontroller_iphone_x_portrait_ios_12 1 3x

To-do:

  • Add Snapshot Test
  • Changelog entry.

/cc @mapbox/navigation-ios

@JThramer JThramer added bug Something isn’t working user: feedback UI Work related to visual components, Android Auto, Camera, 3D, voice, etc. labels Feb 19, 2019
@JThramer JThramer self-assigned this Feb 19, 2019
Copy link
Contributor

@frederoni frederoni left a comment

Choose a reason for hiding this comment

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

An additional view to add some padding seems excessive. Can we constrain the bottom element in the bottom banner with safeBottomAnchor?

Also, the NightStyle inherits from the DayStyle, so we need an appropriate color there as well. Can the padding view’s background color be transparent? Much like the UIStackView, a padding view should be a non-rendering view. However, preferably, we shouldn't have a padding view at all.

@JThramer
Copy link
Contributor Author

Posterity: after talking with @frederoni, we decided to make the bottom padding area a BottomBannerView type. That way, the padding area can stay consistent with the banner, and we won't have to add extra stuff to the style.

@JThramer JThramer marked this pull request as ready for review February 25, 2019 23:34
@JThramer JThramer dismissed frederoni’s stale review February 25, 2019 23:35

We chatted about this, compromised on one type for both views (banner and padding)

CHANGELOG.md Outdated Show resolved Hide resolved
@JThramer JThramer merged commit eaccd28 into master Mar 5, 2019
@JThramer JThramer deleted the jerrad/bottom-banner-safe-area branch March 5, 2019 00:10
@1ec5 1ec5 added this to the v0.30.0 milestone Mar 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn’t working UI Work related to visual components, Android Auto, Camera, 3D, voice, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants