Skip to content

Comments

Fix attribution and logo layout#288

Merged
bsudekum merged 3 commits intomasterfrom
fred-attribution-layout
Jun 20, 2017
Merged

Fix attribution and logo layout#288
bsudekum merged 3 commits intomasterfrom
fred-attribution-layout

Conversation

@frederoni
Copy link
Contributor

@frederoni frederoni commented Jun 20, 2017

Fixes #236

Besides moving the ornaments, this PR also changes the partially revealed height from 75% to 60% of the screen height.

@bsudekum 👀

@frederoni frederoni requested a review from bsudekum June 20, 2017 06:19
@frederoni frederoni force-pushed the fred-attribution-layout branch from 6e8e48a to cd46d8f Compare June 20, 2017 06:25
var arrowCurrentStep: RouteStep?
var isInOverviewMode = false

let overviewContentInset = UIEdgeInsets(top: 65, left: 15, bottom: 55, right: 15)
Copy link
Contributor

Choose a reason for hiding this comment

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

Overview mode required some extra padding on all sides to account for the marker height. How does it look right now?

@bsudekum
Copy link
Contributor

Looks good other than in this mode, two things:

  • The 17m is clipped
  • This image is also in overview mode. When we try and fit the bounds in this view, it zooms all the way out it looks like.

simulator screen shot jun 20 2017 9 53 23 am

@frederoni
Copy link
Contributor Author

frederoni commented Jun 20, 2017

@bsudekum good catch. The overview content insets have been reverted.
The overlapping was an iOS 11 beta issue. It's the exact opposite but let's not make an exception for the initial beta since it's still very flaky.

Here's another iOS 11 shot for demonstration purposes:

@bsudekum bsudekum merged commit f6b6557 into master Jun 20, 2017
@bsudekum bsudekum deleted the fred-attribution-layout branch June 20, 2017 11:47
@1ec5 1ec5 added this to the v0.5.0 milestone Jul 9, 2017
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.

3 participants