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

Public StepsListViewController #1633

Merged
merged 5 commits into from
Sep 6, 2018
Merged

Public StepsListViewController #1633

merged 5 commits into from
Sep 6, 2018

Conversation

bsudekum
Copy link
Contributor

@bsudekum bsudekum commented Aug 29, 2018

This makes it possible to have the same steps list view functionality, that is found in the drop in UI, in a custom navigation UI. I think this is about the least intrusive change possible to have this work.

  • document

/cc @mapbox/navigation-ios

controller.view.topAnchor.constraint(equalTo: instructionsBannerView.bottomAnchor).isActive = true
controller.view.leadingAnchor.constraint(equalTo: view.leadingAnchor).isActive = true
controller.view.bottomAnchor.constraint(equalTo: view.bottomAnchor).isActive = true
controller.view.trailingAnchor.constraint(equalTo: view.trailingAnchor).isActive = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not a fan of how much boilerplate is necessary to make the slide action happen. @frederoni is there anything we can do to encapsulate this better?

Copy link
Contributor

@frederoni frederoni Aug 29, 2018

Choose a reason for hiding this comment

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

You could make UIView.pinInSuperview() public and replace these 4 lines w/
controller.view.pinInSuperview(). But pinInSuperview doesn't respect RTL so make sure you change left->leading and right->trailing in UIView.pinInSuperview()’s implementation if you decide to go that way.

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.

LGTM. Rebase/merge master to make CI go ✅

@bsudekum bsudekum merged commit a3018c3 into master Sep 6, 2018
@bsudekum bsudekum deleted the public-steps branch September 6, 2018 16:55
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.

2 participants