Sylvain Wallez wrote:
Marc Portier wrote:
<snip/>
Sorry for the massive commit, however when walking around the code it only looked like the proverbial tip of the iceberg.
Sorry for the delay, but, as we say here "later is better than never"!
yep, thx for chiming in
> - left quite some TODO markers for next sweaps
Maybe some of you have some suggestions on some of them, feel free to step in and comment:
1/ should getWidget(id) be removed from Widget? It is already on ContainerWidget (which is the true context that makes sense IMHO)
+1 from a theoretical POV, but -1 from a practical one! This will lead to many casts to traverse a widget tree, e.g.
form.getWidget("choice").getWidget("union").getWidget("foo")
will become
((UnionWidget)((ChoiceWidget)form.getWidget("choice")).getWidget("union")).getWidget("foo")
aargh, did this already
Or we may extend getWidget() so that it accepts a path (dotted notation) instead of a simple name, which would allow e.g.
form.getWidget("choice.union.foo")
makes sense, but I haven't seen that much so deep nested structures yet, but surely one we could add to the virtual todo list :-)
2/ should getNamespace() exist at all, it seems to return the same thing as getFullQualifiedId()? Maybe a historical idea waiting to get thrown out?
+1 to remove
done
3/ can getId() ever return null or "" on a widget instance? Can't we carefully asume programming error and allow for the accidental NPE to be thrown
4/ same question on getDefinition()
What's the need for getDefinition() for users of a widget? I consider this as an implementation concern of widgets and would remove it from the public API (i.e. make it protected in AbstractWidget).
I had the same feeling in fact, but didn't go so far yet. would need to check consequences
5/ should we rename ContainerDelegate to simply WidgetList (and the ContainerDefintionDelegate to WidgetDefinitionList)
WidgetList is more understandable than ContainerDelegate ;-)
was done
6/ union seems to generate fi:field in stead of fi:union this surprised me a bit, is that the goal?
+1 for fi:field as, for the view, it isn't different from a field and avoids duplicating the styling (the same is already done for action, submit, repeater-action and row-action)
thx for the extra info, tim already explained the first part
7/ should validation stop as soon as possible or continue to allow all validation errors to be set?
Continue to get all validation errors.
yeah, that seems to be the consensus, I've dropped the event/validation/lifecycle stuff for the moment and am focussing on the @extend/@define reuse first
I'll pick this up later by making a catalogue of the validation stuff now (I thought I saw some inconsistency to the above rule, but the special cases might make sense after all, just need some detail look, discussion and additional dev-docs IMO)
8/ setParent() on abstractWidget should be write-once IMHO, possibly yielding RTE (IllegalState?) when someone tries to reset it
Don't know... A sure thing is that definitions don't need a parent (also they should be immutable). As for reparenting widgets, I don't know if there are some valid use cases...
haven't touched this yet
removing it on the widget-definition is part of my current work on doing the @extend @defines stuff
9/ should not all generateSAXFragments include the getDefinition.generateDisplayData() by default
+1
done that
And I would add:
10/ Split generateSAXFragment() into startSAXFragment() and endSAXFragment(), which will make it so much easier on the view side to handle custom SAX fragments for container widgets and embedding of the <wi:styling>.
hm, actually since the start/end is always grouped and quite similar to all widgets I've made the spilt slightly different:
generateSAXFragment will do the start/end of the containing element, by asking getXMLElementName() and getXMLElementAttributes from the derived concrete subclass
inserting other stuff in between is done by subclassing generateItemSAXFragment
hope you can live with that to get the same flexibility?
(the only flexibility you loose imho is the ability to produce not welformed XML by mismatching your end and start events :-))
Note that I'd like also that <wi:styling> could be written in the definition also, as defining the styling in the widget definition can be a productivity boost with widget repositories!
maybe we could just treat it like the display-data?
(we made that move to the wi namespace as well, so it doesn't seem to unlogic, no?)
wdyt? -marc= -- Marc Portier http://outerthought.org/ Outerthought - Open Source, Java & XML Competence Support Center Read my weblog at http://blogs.cocoondev.org/mpo/ [EMAIL PROTECTED] [EMAIL PROTECTED]
