On Wed, Sep 10, 2008 at 10:01 AM, Miguel Méndez <[EMAIL PROTECTED]> wrote:

>
> On Wed, Sep 10, 2008 at 9:53 AM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>
>> On Wed, Sep 10, 2008 at 9:32 AM, Miguel Méndez <[EMAIL PROTECTED]>wrote:
>>
>>> On Tue, Sep 9, 2008 at 11:17 AM, Eric Ayers <[EMAIL PROTECTED]> wrote:
>>>
>>>> Hello Miguel,
>>>>
>>>> I would like for you to review the attached patch.
>>>>
>>>> This change converts the Size and Icon classes to JavaScript overlays,
>>>> meaning you must use the 'newInstance()' static method to construct them.  
>>>> I
>>>> did them together because there were inter-dependencies.
>>>>
>>>> I also broke up the Marker test and Icon test into two separate test
>>>> cases.  The getImageMap() and setImageMap() now have a JsArray version and
>>>> an int[] version.
>>>>
>>>
>>>> M
>>>> maps/maps/src/com/google/gwt/maps/client/control/ControlPosition.java
>>>>
>>> LG - We do plan on converting ControlPosition as well right?
>>>
>> We can, but it is only used once or twice per map, so I didn't tag it as
>> high priority.
>>
>
> Ok.
>
>
>>>> M      maps/maps/src/com/google/gwt/maps/client/overlay/Polygon.java
>>>>
>>> 54 - typo "creae" -> "create"
>>>
>> Fixed.
>>
> Ok.
>
>
>>
>>
>>>
>>>> M      maps/maps/src/com/google/gwt/maps/client/overlay/Icon.java
>>>>
>>> LG with one nit: 41 - why do we set the anchor to 0,0 for newInstance()?
>>>
>>
>> The other constructor had a comment which I copied here:
>>
>> // Workaround for problem in the Maps API - issue 124
>>
>> Issue 124 has to do with exceptions from the JS API if the anchor is not
>> set.
>>
> Ok.
>
> LGTM
>

Thanks for the review.  Committed as r770.



-- 
Eric Z. Ayers - GWT Team - Atlanta, GA USA
http://code.google.com/webtoolkit/

--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---

Reply via email to