[mapguide-internals] Re: For Review: RFC71

Chris Claydon chris.claydon at autodesk.com
Wed Mar 17 11:59:18 EDT 2010


Hi Jackie,

I spent some time looking at your code yesterday, and it looks really good. I have a couple of comments...

In each version of getselectedfeatures.xxx you have a method called GetPropertyValueFromFeatureReader. The Java version appends an empty string to the end of each value. E.g.

value = reader.GetDouble(propName) + "";

Is this to convert the type to a String? If so, you could use String.valueOf(double d) or Double.toString(double d) to perform this conversion. However, a better approach (for all three languages) might be to format the string with a specified locale, number of decimal places etc. In .NET, you can use String.Format for this. Java has String.format. In PHP, I think you'd use sprintf.

The rest looks good. This is a great addition to the product!

Chris.

-----Original Message-----
From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Jackie Ng
Sent: Wednesday, March 17, 2010 2:24 AM
To: mapguide-internals at lists.osgeo.org
Subject: [mapguide-internals] Re: For Review: RFC71


Ok I figured out the webkit situation, the top level mouse event handler was
smothering the events from the select drop downs. As I have noticed that any
sub-element has a custom onmousedown handler that suppresses event
propagation, I added one such handler for my dropdowns and now I can see the
options in both dropdowns for all tested browsers.

I now consider this implementation complete and ready for inclusion into the
trunk and the 2.2 branch. But I'd still like a final review of my changes
before doing this.

- Jackie
-- 
View this message in context: http://n2.nabble.com/For-Review-RFC71-tp4736647p4748900.html
Sent from the MapGuide Internals mailing list archive at Nabble.com.
_______________________________________________
mapguide-internals mailing list
mapguide-internals at lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/mapguide-internals


More information about the mapguide-internals mailing list