intuit / CardParts

A reactive, card-based UI framework built on UIKit for iOS developers.
Other
2.52k stars 224 forks source link

Add new CardPartMapView #185

Closed mvdmakesthings closed 4 years ago

mvdmakesthings commented 4 years ago

Related to https://github.com/intuit/CardParts/issues/183

Alright! Putting myself out there for the first time. Here's my first stab at it. I added three different properties that can be accessed via the RX binding syntax:

I also made sure the the underlying MKMapView was available so that users could access its delegate functions or interact with the map directly as needed.

In my example, I showed how you could bind directly to a CardPartTextField's value and change the location of the map. At the same time, I'm cycling through different "zoom levels".

I'm totally open to adding more bindable properties if you think it needs more.


On a personal note, this was a lot of fun and this library is very well organized. Well done to all of the other contributors that have worked on this project.

badrinathvm commented 4 years ago

Below code can be simplified even more cleaner

public override func updateConstraints() {
    mapView.layout {
         $0.top == self.topAnchor
         $0.leading == self.leadingAnchor
         $0.trailing == self.trailingAnchor
         $0.bottom == self.bottomAnchor
      }
        super.updateConstraints()
    }

This layout{} method also sets translatesAutoResizing to false as well..

croossin commented 4 years ago

Awesome work, @codethebeard!

I think a few potentially awesome properties to expose as bindable would be:

It's awesome that you made the mapView public so that the developer can easily access it whenever needed.

Again, we appreciate the contributions and are open to hear about any other features you'd like to see in CardParts. Thanks!

mvdmakesthings commented 4 years ago

@croossin Thanks man! I appreciate the opportunity. I can certainly add those properties.

mvdmakesthings commented 4 years ago

@croossin

I think a few potentially awesome properties to expose as bindable would be:

  • Annotations
  • userLocation

So I just started looking at this and realized that both the MKMapView.annotations [MKAnnotation] and userLocation MKUserLocation properties are get-only properties and probably wouldn't work as bindable properties.

I could potentially expose them as a ControlProperty possibly which might be able to give you some mechanism to observe on those changes, however in the case of userLocation the recommended way to obtain that value is to implement the MKMapView's mapView(_: MKMapView, didUpdate: MKUserLocation) delegate method which would limit the developers ability to safely implement the MKMapView delegate themselves. (basically overriding the delegate)

Any thoughts? I'm a little new to this level of RX but would love to learn a thing if there's a way to properly do what you're asking.

croossin commented 4 years ago

@codethebeard What if for the annotations we used addAnnotations([MKAnnotation]) and removeAnnotations([MKAnnotation])? My only concern would be performance.

Maybe we take a look at how RxMKMapView deals with annotation binding.

I just felt that binding annotations to the mapview would be a key request, but we can discuss if we want to have that as a "to-do" feature.

mvdmakesthings commented 4 years ago

@croossin Yeah, I can see a lot of people wanting that type of feature. I'll start taking a look at the RxMKMapView when I get some free time today to see what they are doing and report back.

croossin commented 4 years ago

@codethebeard Awesome! I'm sure we can figure out this problem 🤓

mvdmakesthings commented 4 years ago

@croossin So, I took a look at RxMKMapView to see how they are doing the annotation binding. I gave it the ol' college try but that rabbit hole goes pretty deep and it's honestly a little over my head. Maybe someone out there in the community with more Swift/RxSwift experience and time could be able to make this Card Part what it really needs to be.

croossin commented 4 years ago

No worries at all @codethebeard. Awesome work thus far. I will go ahead and merge/release this tomorrow! We can add that support later on.

Feel free to email (chase_roossin@intuit.com) your t-shirt size as well as mailing address if you’d like some Intuit swag for helping participate and contribute during Hacktoberfest! We really appreciate all the work.