Roald,

thanks for your efforts. Nice piece of work!

Roald de Wit wrote:
> - LayerMetaData is a fully separate widget and 'registers itself' trough
> a model.setParam("layerMetaData"). LayerControl checks for that param
> and shows the 'i' button if set to true.
>   

I like that approach. Only one minor thing: from the JSDoc comment of 
LayerMetaData.paint, it should be more clear how the params object looks 
like. The current JSDoc looks like the function has two parameters 
layerNode and domNode, but in fact there is objRef and params.

> - CatalogSearchForm has no more hardcoded references. They all are
> configurable through config.xml. I also used the this.getProperty
> function (hopefully properly).
>   

Yes, you use getProperty properly. The targetContext property is 
intended to point to the map context a widget works on. So you should 
use a different name for that property, because it points to something 
different. Another minor thing: I would suggest you remove unused config 
properties from config.xml.

> - SaveContext.js is now Save.js and the listener is placed in a init()
> function.
>   

That's perfect.

> Issues:
> - expanded metadata div collapses on map update
>   

So you should maybe store the state of the div in the widget and restore 
it on update.

> - maybe naming of properties in widgets is not too intuitive (especially
> CatalogSearchForm), please check them.
>   

I'll have another look at those once you remove unused properties from 
config.xml. Maybe some comments on what those are for would be good, 
because right now I cannot really figure out what those are all for.

> - some CSS might need to be moved to the default css
>   

CSS that is needed for rendering of widgets should go in the default 
css, CSS that is used to style your example not.

Again, great work, and we're almost there.

Thanks!
Andreas.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
mapbuilder-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/mapbuilder-devel

Reply via email to