LGTM! On Thu, Sep 18, 2008 at 3:49 PM, Eric Ayers <[EMAIL PROTECTED]> wrote:
> Updated patch attached. > > On Thu, Sep 18, 2008 at 3:08 PM, Miguel Méndez <[EMAIL PROTECTED]> wrote: > >> A >> maps/maps/src/com/google/gwt/maps/client/event/EarthInstanceHandler.java >> LG >> >> M maps/maps/src/com/google/gwt/maps/client/impl/MapImpl.java >> LG >> > Removed hideControls() and showControls() from the patch - what were those > methods? I didn't see them documented. > > >> >> M maps/maps/src/com/google/gwt/maps/client/MapWidget.java >> 1408 - Javadoc? >> > Added. > > >> >> M >> >> maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/HelloMaps.java >> LG >> >> A >> >> maps/samples/hellomaps/src/com/google/gwt/maps/sample/hellomaps/client/EarthPluginDemo.java >> 77 - This code won't work on Java 1.5, remove the @Override. >> > Fixed. > > >> Should we run the demo on a non-windows box? >> > > If you are asking if I've tested this on a non-windows box, the answer is > "yes". It prints a nice "not supported" or "click here to download" if your > browser is not supported or it discovers you just don't have the plugin > installed. > > I don't want to put in code to remove the method on other machines - you > can test to see if the plugin loaded properly (the callback returns null), > and it could be that in the future the plugin folks will support more > platforms. > > > >> On Thu, Sep 18, 2008 at 2:51 PM, Eric Ayers <[EMAIL PROTECTED]> wrote: >> >>> >>> Hello Miguel, >>> Attached is a modified version of the patch was originally put forth by >>> Jesse Crossley to address issue 128 & 133. >>> >>> http://code.google.com/p/gwt-google-apis/issues/detail?id=128 >>> http://code.google.com/p/gwt-google-apis/issues/detail?id=133 >>> >>> I've modified it to use the "Handler" pattern and added a demo. If you >>> want to see it work, run it on a Windows platform, that's the only platform >>> supported by the Google Earth plugin. >>> >>> -Eric. >>> >>> On Thu, Jul 17, 2008 at 9:03 AM, Miguel Méndez <[EMAIL PROTECTED]>wrote: >>> >>>> I think that I drew the *Handler analogy with the assumption that the >>>> spirit of the change would shine through. I fired that email off a little >>>> quickly -- my bad. >>>> What I meant to suggest was that the Java callback interface should >>>> provide some insulation from changes to the signature of the JS callback >>>> interface. I did not mean that we needed to add a handler collection et >>>> al. >>>> Something like this would suffice: >>>> >>>> interface EarthInstanceCallback { >>>> class EarthInstanceCallbackArguments { >>>> EarthInstance getEarthInstance() { >>>> return ... >>>> } >>>> } >>>> void onEarthInstance(EarthInstanceCallbackArguments args); >>>> } >>>> >>>> That way changes to the JS earth callback method signature would result >>>> in the deprecation or addition of members to the >>>> EarthInstanceCallbackArguments class. I suggested this because there have >>>> been several times where the JS API has changed the signature of the the JS >>>> callback methods. Given that the Earth/Maps integration is relatively new >>>> I >>>> would expect some volatility. >>>> >>>> >>>> On Wed, Jul 16, 2008 at 8:26 PM, jesse crossley < >>>> [EMAIL PROTECTED]> wrote: >>>> >>>>> okay, i think i've got something more in the spirit of the Handler >>>>> pattern and that will proof against future google-maps-api changes. patch >>>>> is attached. i think, though, that some extra mods have slipped in to >>>>> MapWidget and MapWidgetImpl, where i was exposing and playing with methods >>>>> i'd found on the GMap2 JavaScript object. >>>>> >>>>> >>>>> >>>>> On Wed, Jul 16, 2008 at 4:10 PM, jesse crossley < >>>>> [EMAIL PROTECTED]> wrote: >>>>> >>>>>> We are manufacturing the event types. An event type is really just a >>>>>>> container to hold the event arguments. The intention is to future >>>>>>> proof the >>>>>>> gwt-google-apis callbacks against changing JS apis that add or remove >>>>>>> arguments. That is just fine in JS but wreaks havoc on a Java API >>>>>>> (dynanic >>>>>>> vs. strong typing) >>>>>>> >>>>>> >>>>>> so would you envision a public abstract static class >>>>>> EarthInstanceCallback (defined in EventImpl.java)? and would we also add >>>>>> EARTHINSTANCEINITIALIZED("earthinstanceinitialized") and >>>>>> EARTHINSTANCEFAILED("earthinstancefailed") to the MapEvent enumeration? >>>>>> >>>>>> after looking closer, that seems wrong since we're not attaching any >>>>>> proper listeners to anything, so the Callback as defined in EventImpl >>>>>> seems >>>>>> incorrect. >>>>>> >>>>>> >>>>>>> I think the important bit we want to copy from the Handler pattern is >>>>>>> the creating of a container to hold the callback arguments returned >>>>>>> from the >>>>>>> JS callback, and then passing the container as the only argument to the >>>>>>> user >>>>>>> callback. Then, you can add methods to the event object like >>>>>>> getEarthInstance() and isValid() or something like that. This future >>>>>>> proofs >>>>>>> this method against JS API changes. >>>>>>> >>>>>> >>>>>> i think what's giving me grief is trying to match Callback and Handler >>>>>> terminology. it seems like we would want a public interface >>>>>> EarthInstanceCallback that behaves like the other >>>>>> com.google.gwt.maps.event.***Handlers, like this: >>>>>> >>>>>> >>>>>> public interface EarthInstanceCallback { >>>>>> class EarthInstanceEvent extends EventObject { >>>>>> public EarthInstanceEvent(Earth source) { >>>>>> super(source); >>>>>> } >>>>>> public Earth getSender() { >>>>>> return (Earth) getSource(); >>>>>> } >>>>>> } >>>>>> void onInitialized(EarthInstanceEvent event); >>>>>> void onFailed(EarthInstanceEvent event); >>>>>> } >>>>>> >>>>>> with the understanding that EarthInstanceEvent.getSender() will return >>>>>> null if onFailed(EarthInstanceEvent) is invoked. >>>>>> would we want this interface to be called "EarthInstanceCallback" to >>>>>> match the google-maps-api terminology, or should it be >>>>>> "EarthInstanceHandler", in keeping with the gwt-maps Handler pattern? >>>>>> >>>>>> if i implement this, then i think the MapWidget will no longer have an >>>>>> Earth field; it will just invoke the GMap2.getEarthInstance(callback) >>>>>> method >>>>>> everytime. ah, but i'll still want to pass in my own callback function >>>>>> to >>>>>> GMap2 that will then invoke the right method on the EarthInstanceHandler >>>>>> (or >>>>>> EarthInstanceCallback, whatever it's named). >>>>>> >>>>>> lemme try it out... >>>>>> >>>>>> >>>>>> >>>>> >>>> >>>> >>>> -- >>>> Miguel >>>> >>> >>> >>> >>> -- >>> Eric Z. Ayers - GWT Team - Atlanta, GA USA >>> http://code.google.com/webtoolkit/ >>> >> >> >> >> -- >> Miguel >> > > > > -- > Eric Z. Ayers - GWT Team - Atlanta, GA USA > http://code.google.com/webtoolkit/ > -- Miguel --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
