Re: Markers and Popovers

[Jonas Danielsson <the sator gmail com>]

On Thu, May 22, 2014 at 01:18:43AM +0200, Mattias Bengtsson wrote:
> Great work Damián!

Yeah, good work. I will add some comments below.

> Moving this to the maps-list since I think it deserves to be public. If
> you're not all subscribed yet, do it now please! :)
>
> (Also please forward your previous replies and keep further discussion
> on the list! :))
>
> On ons, 2014-05-21 at 13:29 -0300, Damián Nohales wrote:
> > Hi guys,
> >
> > I was racking my brain to get a class design for markers and popovers
> > in Maps. My idea is that the markers and popovers were versatile so
> > anyone can have the appearance that anyone wants, and that adding new
> > kind of markers could be neat and well separated from the other
> > components.
>
> That sounds good!
>
> > I think one thing missed here is where in the code and how render the
> > marker, there is a note in the top left of the diagram that explain
> > the problem and some differences in the current markers. Also is
> > missed some connection between the markers/popovers with the rest of
> > the UI components.
> >
> > Please take a look on this, any suggestion will be appreciated.

I do not see it as much of a problem. The renderer will add a marker
to the content of the tile along with some kind of callback on button-press
or notify::selected or whichever signal is best. And in the callback we will
have access to the marker. What would be the real difference between a marker
that is accessed in that way and one that is created on a markerlayer?

And how would it affect the positioning algorithm? We will still have access
to lat/lon x/y.


> Regarding the UML:
> It was a while ago that I did any class diagram stuff (basically just
> for a course some 7-8 years ago). 
> But according to a reference I quickly read now[1]:
>  + means public
>  - means private
>  # means protected
>
> It would be great if you could state if the properties are part of the
> API or not by adding -, + or # in front. 
> Also some of the methods looks like they might be protected and not
> private (like createPopover()). Also please add _ before the protected
> and private methods (just so I can understand what you mean exactly :)) 
>
> Some questions of the API:
> 1) why might a MapMarker have many markers? Is this actually when (for
> example) userMarker has several actors? I think this could be solved by
> just special casing userMarker and let it have an
> AccuracyCircleActorSomething in it that is shown when the popover is
> shown. No need to make this part of the API by having a markers field I
> think (if it is, I didn't understand that really, see above).

I think I agree with this.

> 2) If my thoughts are correct in §1, why not just let MapMarker inherit
> from Champlain.Marker? Then you'd get set/get -reactive etc for free. 

And also this. Inheriting from Champlain.Marker or Champlain.CustomMarker
feels like the way to go.

> 3) regarding the current userLocation and mapLocation my guess is that
> we would make them userMarker and mapMarker and then make a
> searchResultMarker that inherits from mapMarker right? 
>
> 4) What is the positioning algorithm?

Yeah, I have the same question. We should always have access to the lat and lon
right? So positioning should just be a matter of getting the x and y through
the ChamplainView and creating a rect?
 
> 5) I'm not sure entirely how the overlay should come to the popover,
> maybe add it back to the mapView and have it there as a property? Really
> not sure.

We could also go with making mapView inherit from GtkOverlay.

> Nitpicks:
> 1) I think you can drop the Generic in front of Poi{Marker, Popover}
> 2) MapMarkerPopover → MapPopover
> 3) Maybe replace all Popover-instances with Bubble instead, since then
> we know that we're talking about the bubbles in the map and not the
> popovers in the headerbar etc.

4)
What is the point of the createUI function of the MarkerPopover?
> > I think after this I'm going to start some test code based on the
> > Rishi branch, so POI can be taken into account during coding.
>
> One possibility would be to instead take a stab at the
> userLocation{marker, bubble}, it is directly related to the check-in
> stuff in your project after all. 
>
> Regards,
> Mattias

/Jonas

> 1:
> http://www.ibm.com/developerworks/rational/library/content/RationalEdge/sep04/bell/