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

[Enhancement] Map with shapes from pin #1673

Closed
kingces95 opened this issue Jan 26, 2018 · 20 comments · Fixed by #6136
Closed

[Enhancement] Map with shapes from pin #1673

kingces95 opened this issue Jan 26, 2018 · 20 comments · Fixed by #6136
Labels
community-sprint F100 help wanted We welcome community contributions to any issue, but these might be a good place to start! inactive Issue is older than 6 months and needs to be retested t/enhancement ➕ up-for-grabs We welcome community contributions to any issue, but these might be a good place to start!
Projects

Comments

@kingces95
Copy link
Contributor

kingces95 commented Jan 26, 2018

Rationale

UX could reasonably want to overly shapes onto maps. For example, a semi-transparent circle centered on a pin.

Implementation

Could make it complicated and general or could just make it this:

public class Pin {
	public static readonly BindableProperty CircleColor; // color
	public static readonly BindableProperty CircleSize; // double
}

Expected Result

A circular map overlay centered at the pin.

Implications for CSS

None.

Backward Compatibility

None.

Difficulty : Easy

@pauldipietro pauldipietro added this to New in Triage Jan 26, 2018
@kingces95 kingces95 added this to Backlog in Enhancements Jan 26, 2018
@hartez hartez removed this from New in Triage Jan 29, 2018
@bmacombe
Copy link
Contributor

I'm willing to take a look at this issue. Would like some others input on what units to use to define the size of the circle. Do we want to just do a simple circle or allow more complex items like polygons or lines based on OpenGIS Well Know Binary/Text, etc?

@kingces95 kingces95 changed the title [Enhancement, Placeholder] Map with circle from pin [Enhancement] Map with circle from pin Feb 5, 2018
@davidortinau
Copy link
Contributor

@bmacombe once the spec is "Ready for Implementation" I can make the assignment.

I agree, a circle is too limited. Could this take any view instead?

I'm looking at examples of other maps and we should be able to handle any manner of view that handles its own color, border, opacity, shape, and even animation.

I think we need some example use cases to guide this. @dotMorten perhaps you have some insight?

@bmacombe
Copy link
Contributor

bmacombe commented Feb 6, 2018

Using a view would be interesting, haven't thought of that angle. I come from a more GIS centric background, so point, lines, polygons described via spatial coordinates is my normal thinking but I could see just sticking a view on a pin as more useful for many apps.

I'm happy to take a look once the spec is ready or glad to let someone else work on it also.

@dotMorten
Copy link
Contributor

I'd be happy to give some insights but I feel like some of the requirements here are missing. Is the circle supposed to show how far something is from the pin? Ie a 1mile radius?
If so I don't really get how a polygon relates to a pin? Where should the pin be (centered doesn't mean the pin is inside).

@andreinitescu
Copy link
Contributor

andreinitescu commented Feb 6, 2018

My suggestion is to add a bindable property to the Map class to be able to customize the pin appearance:

public class Map
{
       public DataTemplate PinTemplate { get; set; }
       public DataTemplateSelector PinTemplateSelector { get; set; }
}

By default it's null, and the Map uses the current pin today.
Furthermore, the PinTemplateSelector property will allow appearance customization by each Pin object.

@dotMorten
Copy link
Contributor

dotMorten commented Feb 6, 2018

I find that terrible limited. I want pins to look different.
TBH I'd rather I could add a set of polygons, polylines and points. Each can have their own styling. It'll give the most flexibility for a bunch of scenarios, and if I want to add a pin with a circle around it, I'll just add one point and one circular polyline. I get the convenience of just having it in one simple pin-class but it's also very limiting and not considering other scenarios.

It's also not clear how the PinTemplate would create a circle of a certain metric radius?

@bmacombe
Copy link
Contributor

bmacombe commented Feb 6, 2018

@dotMorten That is similar to what I was thinking. I've used ArcEngine and ThinkGeo's mapsuite package for many years. I was thinking something like a layer, with geometry features defined in WKB with attributes and styles.

What I wonder is many if the apps implemented in Forms are not doing the complicated GIS, but just need to quickly show locations on a map. Business locations, delivery radius, etc.

It might be that both would best, I would like the fuller GIS features myself...enough that I've already implemented it on UWP, iOS and Android. But I did it early in my time with XF and I don't think the API or code quality is good enough to contribute it directly.

If having the more complicated GIS features is something that will be of interest, I would be glad to propose an API based on my current work for further iteration.

@dotMorten
Copy link
Contributor

@bmacombe I guess you could also argue that this would be taking it too far for a simple map control, and might be hard to make work well cross-platform with the native map controls under them. The better approach might be to just support pins, as this is the most common scenario.
If you then need to do more advanced GIS scenarios, perhaps this isn't the control for you, and you could use something like the <ShamelessPlug>ArcGIS Runtime for Xamarin Forms</ShamelessPlug>.

@bmacombe
Copy link
Contributor

bmacombe commented Feb 6, 2018

@dotMorten Thanks for the Shameless Plug! I didn't know that existed!

@bmacombe
Copy link
Contributor

bmacombe commented Feb 6, 2018

While not reaching the level of complexity and features available from ESRI, it actually is fairly easy to draw simple geometry (points, lines and polygons) on the native map controls. It could still be a useful feature for simple mapping needs.

Just some examples below on the basic conversion from an NTS Polygon to a platform polygon for thought. I left Android out for now...because it's a mess!

iOS
`public static CLLocationCoordinate2D ToClLocationCoordinate2D(this Coordinate coord) =>
new CLLocationCoordinate2D(coord.Y, coord.X);

    public static MKCoordinateRegion ToMkCoordinateRegion(this Envelope envelope)
    {
        var span = new MKCoordinateSpan(envelope.Height, envelope.Width);
        var center = envelope.Centre.ToClLocationCoordinate2D();
        return new MKCoordinateRegion(center, span);
    }

    public static MKPolygon ToMKPolygon(this IPolygon ntsPoly)
    {
        //Create the hole polygons
        var holes = ntsPoly.Holes.Select(h => MKPolygon.FromCoordinates(ToClLocationCoordinate2Ds(h))).ToArray();

        //Create the ring polygon
        return MKPolygon.FromCoordinates(ntsPoly.Shell.ToClLocationCoordinate2Ds(), holes);
    }

    public static CLLocationCoordinate2D[] ToClLocationCoordinate2Ds(this ILineString linestring)
    {
        return linestring.Coordinates.Select(c => c.ToClLocationCoordinate2D()).ToArray();
    }

`

uwp
` public static Geopoint ToGeopoint(this Coordinate c) =>
new Geopoint(c.ToBasicGeoposition());

    public static BasicGeoposition ToBasicGeoposition(this Coordinate c) =>
        new BasicGeoposition() { Latitude = c.Y, Longitude = c.X, Altitude = c.Z };

    public static Geopath ToGeopath(this ILineString lineString) =>
        new Geopath(lineString.Coordinates.Select(c => c.ToBasicGeoposition()));

    public static GeoboundingBox ToGeoboundingBox(this Envelope envelope)
    {
        return new GeoboundingBox(new BasicGeoposition() { Latitude = envelope.MaxY, Longitude = envelope.MinX },
            new BasicGeoposition() { Latitude = envelope.MinY, Longitude = envelope.MaxX });
    }

    public static MapPolygon ToMapPolygon(this IPolygon polygon)
    {
        var newPoly = new MapPolygon();
        newPoly.Paths.Add(polygon.Shell.ToGeopath());
        foreach( var geopath in polygon.Holes.Select(p => p.ToGeopath()) )
            newPoly.Paths.Add(geopath);
        return newPoly;
    }`

@samhouts
Copy link
Member

Would anyone on this thread care to officially take on this enhancement and submit a pull request for it? Thanks!

@samhouts samhouts added the help wanted We welcome community contributions to any issue, but these might be a good place to start! label Jun 14, 2018
@bmacombe
Copy link
Contributor

I could probably take a look at it, but I'm fairly busy at the moment. Not sure how quickly I would get it done.

Is there any decision on if to just do the simple color and radius around a pin or something else?

@dotMorten
Copy link
Contributor

@bmacombe I think the better and more flexible option is to allow people to draw a polygon and/or polyline, and not focus so much on a radius circle. You could use the polygon approach to create that circle.

@bmacombe
Copy link
Contributor

bmacombe commented Jun 20, 2018

@dotMorten To be clear are you thinking let them draw a polygon in screen coordinates and just draw that or in map coordinates?

Edit: Or I should say in screen units or map units, didn't mean coordinates

@dotMorten
Copy link
Contributor

Map coordinates. Screen units makes no sense in this context.

@samhouts samhouts moved this from Backlog to Ready for Implementation in Enhancements Jul 26, 2018
@samhouts samhouts added the up-for-grabs We welcome community contributions to any issue, but these might be a good place to start! label Oct 5, 2018
@andreinitescu
Copy link
Contributor

andreinitescu commented Nov 13, 2018

The ticket needs a spec clearly defined. Also, the title should be changed to reflect the need to display shapes on the map as described in the Rationale section.

Here's my take:

New members in the Map class:

public class Map
{
   public event EventHandler<MapElement> MapElementClicked;
   public IList<MapElement> MapElements { get; }
}

The MapElements property allows setting the map elements in XAML or by code:

<Map>
   <Map.MapElements>
      <MapCircle />
      <MapPolyline />
      <MapPolygon />
   <Map.MapElements>
<Map>
Map map = ...;
map.MapElements.Add(new MapCircle());
map.MapElements.Add(new MapPolyline());
map.MapElements.Add(new MapPolygon());

The elements are implemented like the following:

// Base class for map elements, more common properties can go here
public class MapElement : BindableObject
{
   public bool IsVisible { get; set; }
}

public class MapCircle : MapElement
{
   public Color FillColor { get; set; }
   public bool Position Center { get; set; }
   public Distance Radius { get; set; }
   public Color StrokeColor { get; set; }
   public double StrokeThickness { get; set; }
}

public class MapPolyline: MapElement
{
   public Geopath Path { get; set; }
   public Color StrokeColor { get; set; }
   public double StrokeThickness { get; set; }
}

public class MapPolygon : MapElement
{
   public Color FillColor { get; set; }
   public IList<Geopath> Holes { get; set; }
   public Geopath Outline { get; set; }
   public Color StrokeColor { get; set; }
   public double StrokeThickness { get; set; }
}

public class Geopath : BindableObject
{
   public IEnumerable<Position> Positions { get; set; }
}

Once the spec is approved I could try take a stab at it.

@CZEMacLeod
Copy link

@andreinitescu I'm not sure you need both Polyline and Polygon, shapes can be filled or not by FillColor = Color.Transparent and the Geopath can be set to AutoClose or not to differentiate a path and an enclosure.
I also think perhaps there is a use case for including a MapRectangle where you set the Top, Left, Bottom and Right and it generates the appropriate Geopath for you, although maybe that is easy enough to implement using a bound model rather than an actual viewable element.

@andreinitescu
Copy link
Contributor

@CZEMacLeod usually transparent background means it's still clickable. Someone might want some lines drawn but without having the interior clickable.

@CZEMacLeod
Copy link

@andreinitescu I think that argument can be made against the MapCircle - I can think of many instances where you want the circle itself to be clickable, but the interior not, such that you can click places or other locations, pins etc. inside. (e.g. Restaurants within X miles of your current location)

@andreinitescu
Copy link
Contributor

I think it's better to have polyline and polygon separate, all platforms have it like this.

@rmarinho rmarinho changed the title [Enhancement] Map with circle from pin [Enhancement] Map with shapes from pin Nov 13, 2018
@samhouts samhouts added this to In Progress in v4.1.0 May 8, 2019
@samhouts samhouts removed this from Ready for Implementation in Enhancements May 9, 2019
@samhouts samhouts added this to In Progress in v4.2.0 May 29, 2019
@samhouts samhouts removed this from In Progress in v4.1.0 May 29, 2019
@samhouts samhouts added the inactive Issue is older than 6 months and needs to be retested label Jul 4, 2019
@samhouts samhouts removed this from In Progress in v4.2.0 Jul 4, 2019
@samhouts samhouts added this to In Progress in v4.3.0 Jul 8, 2019
v4.3.0 automation moved this from In Progress to Done Aug 29, 2019
samhouts pushed a commit that referenced this issue Aug 29, 2019
* Add Polyline to Map

* Android Polyline rendering

* iOS polyline renderer

* UWP polyline renderer

* Unregister OnPolylineCollectionChanged in Dispose of UWP renderer

* Add MapElement base class

* Update Android MapRenderer with MapElement

* Update UWP MapRenderer with MapElement

* Update iOS MapRenderer with MapElement

* Add polygons

* Tweak functionality of gallery page

* Rename some things in Android renderer

* Made LoadPolyline and LoadPolygon virtual on UWP

* Fix iOS/Mac MapRenderer compile errors
fixes #1673
fixes #5773
@samhouts samhouts added this to In Progress in v4.4.0 Aug 30, 2019
@samhouts samhouts moved this from In Progress to Done in v4.4.0 Aug 30, 2019
@samhouts samhouts removed this from Done in v4.3.0 Sep 3, 2019
@samhouts samhouts removed this from Done in v4.4.0 Sep 11, 2019
@samhouts samhouts added this to In Progress in v4.3.0 Sep 11, 2019
@samhouts samhouts moved this from In Progress to Done in v4.3.0 Sep 11, 2019
felipebaltazar pushed a commit to felipebaltazar/Xamarin.Forms that referenced this issue Oct 16, 2019
* Add Polyline to Map

* Android Polyline rendering

* iOS polyline renderer

* UWP polyline renderer

* Unregister OnPolylineCollectionChanged in Dispose of UWP renderer

* Add MapElement base class

* Update Android MapRenderer with MapElement

* Update UWP MapRenderer with MapElement

* Update iOS MapRenderer with MapElement

* Add polygons

* Tweak functionality of gallery page

* Rename some things in Android renderer

* Made LoadPolyline and LoadPolygon virtual on UWP

* Fix iOS/Mac MapRenderer compile errors
fixes xamarin#1673
fixes xamarin#5773
felipebaltazar pushed a commit to felipebaltazar/Xamarin.Forms that referenced this issue Oct 16, 2019
* Add Polyline to Map

* Android Polyline rendering

* iOS polyline renderer

* UWP polyline renderer

* Unregister OnPolylineCollectionChanged in Dispose of UWP renderer

* Add MapElement base class

* Update Android MapRenderer with MapElement

* Update UWP MapRenderer with MapElement

* Update iOS MapRenderer with MapElement

* Add polygons

* Tweak functionality of gallery page

* Rename some things in Android renderer

* Made LoadPolyline and LoadPolygon virtual on UWP

* Fix iOS/Mac MapRenderer compile errors
fixes xamarin#1673
fixes xamarin#5773
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-sprint F100 help wanted We welcome community contributions to any issue, but these might be a good place to start! inactive Issue is older than 6 months and needs to be retested t/enhancement ➕ up-for-grabs We welcome community contributions to any issue, but these might be a good place to start!
Projects
No open projects
v4.3.0
  
Done
Development

Successfully merging a pull request may close this issue.

7 participants