branflake2267 / GWT-Maps-V3-Api

GWT Maps V3 Javascript Bindings
Other
144 stars 113 forks source link

Fix for getControls #115

Closed schweighart closed 11 years ago

schweighart commented 11 years ago

Added getControls for a single controlPosition. Since setControls uses controls[controlPosition] to add a control, getControl should do the same.

branflake2267 commented 11 years ago

Hm, hard to see the difference.

schweighart commented 11 years ago

My tools shows the diff perfect, but github behaves a bit strange. I don't know what exactly has gone wrong since it's the first time I commited something to github.

To MapWidget.java I added the following lines:

    /**
     * returns all controls for a controlPosition
     * @param controlPosition
     * @return
     */
    public MVCArray<Element> getControls(ControlPosition controlPosition) {
        return impl.getControls(controlPosition);
    }

To MapImpl.java I added the following lines:

    /**
     * returns all controls for a controlPosition
     * @param controlPosition
     * @return
     */
    public final MVCArray<Element> getControls(ControlPosition controlPosition) {
        return getControls(controlPosition.value());
    }

    /**
     * returns all controls for a controlPosition
     * @param controlPosition
     * @return
     */
    private final native MVCArray<Element> getControls(int controlPosition) /*-{
        return this.controls[controlPosition];
    }-*/;

And in MapImpl.java I changed the getControls() method to return this.controls; instead of controls;

branflake2267 commented 11 years ago

Thanks so much for the source! Nice job.

yeah, I find github does this too.

branflake2267 commented 11 years ago

I'd like to write a test case for this before i merge it. Unless you'd like to do that. Anyway, I can work on this tonight.

schweighart commented 11 years ago

Unfortunately I currently have a lot of errors in the tests. I think it's not a good idea if I write tests blind :-)

But I have another convenience function which could be added to the MapWidget. I currently have it in a helper class. If it's inside the MapWidget the map parameter can be removed:

    public final void resizeMap(final MapWidget map) {
        final LatLng center = map.getCenter();
        resizeMap(map.getJso());
        map.setCenter(center);
    }

    public final native void resizeMap(MapImpl map) /*-{
        $wnd.google.maps.event.trigger(map, 'resize');
    }-*/;

I'm mentioning it because it took me some time to find out that I have to call resize on the Jso object instead of the MapWidget.

branflake2267 commented 11 years ago

Sounds good, want to add that? Resizing has been a issue a few times. Nice function!

You'll see some red during the test, which is normal, b/c something isn't function during the test, and I didn't go all out an fix it. As long as the junits pass, thats what counts, b/c thats what the test aimed for.

thanks again!

branflake2267 commented 11 years ago

By the way if you write a test, I'll review it, broken or not, which doesn't matter to me. :) Don't worry about what might look broken. Last I tested yesterday, every test was passing.

schweighart commented 11 years ago

I'll add the function and look at the tests. I just saw some exceptions in them and did not look deeper.

branflake2267 commented 11 years ago

if you see red or weird things in the console, you can ignore that information :)

schweighart commented 11 years ago

I added the resize code and 2 unittests. The project looks much better when I work from home, since there is no firewall blocking my internet access :-)

branflake2267 commented 11 years ago

Nice. FYI, I might run out of steam tonight, as to getting this integrated. So I might have to push integration into the next eve. I've got a job interview in 45 minutes and it might wipe me out. :)

schweighart commented 11 years ago

Take your time. :-) I'm still using a snapshot from end of November. Good luck for your job interview.

branflake2267 commented 11 years ago

Thanks, sounds good.

branflake2267 commented 11 years ago

I'm going to work on this in the morning. :)