Towards the larger user-base I think we should unify views, clean out the code and provide appropriate documentation.
So, here is an attempt to bring the sheep back into the heard, and lay down a better foundation for the RepeaterBinding implementation as well as its use (and thus its documentation)
+1
looking into the <fb:repeater> syntax we see these mappings back in the names of the attributes:
@id points to the repeater
@parent-path points to the matching contextbean
the repeater's rows each map to one item found at @row-path within the contextbean
the nested child-widgets will map @id of widget to @path of actual property.
I found the (at that time) wb:repeater syntax really confusing when using it the first time. There were 5 attributes and there usage was not obvious to me: @id, @parent-path, @row-path, @unique-id, @unique-row-path. That was why I liked Antonio's move of the last two attrs into elements so much.
What's still confusing IMO is the name parent-path - the path of which parent? For consistency this should also be named @path as for all other binding elements, the name @row-path avoids any further confusion I think.
[case 1]
DESCRIPTION:
- the form handling (actions or events) will not reorder the rows
- no rows will be added or deleted
- item-properties could be sparsly bound to row-widgets (meaning: not all properties are mapped to actual widgets since they're not part of the foreseen end-user interaction scheme, of course the ones that were not bound should not be cleared on save just because the form-model isn't holding a value for them)
In this case we don't really speak of identity of the items, and we don't really need it either: the position of each item in the list (it's index) serves as an implicit identifier, and since no adding/deleting or reordering on the rows can happen we know that the row-indexes will just keep in sync.
PROPOSED SYNTAX: <fb:repeater id="rep-id" parent-path="." row-path="item"> <!-- no fb:identity --> <!--required elements for this case --> <fb:on-bind>...</fb:on-bind> <!--optional elements for this case --> <fb:on-insert>..</fb:on-insert> <fb:on-delete>..</fb:on-delete> </fb:repeater>
EXPECTATION DETECTION:
- no nested <fb:identity> element tells us we don't have an explicit identity, and we should just trust the index as a identity correlator.
STRATEGY on 'save': - foreach row in repeater - use the nested 'on-bind' binding
A straightforward extension seems to be - allow for add/delete of rows (by adding <on-insert> <on-delete>) - at the cost of not surviving sparse binding of items.
Then the Strategy becomes: - foreach row in repeater with matching index in the items - use the nested 'on-bind' binding - for excess rows in the repeater - use on-insert and on-bind - for excess items in the context - use on-delete
-o0o-
[case 2] (aka current SimpleRepeaterBinding) DESCRIPTION: - no fluff, just stuff - the simplicity of 'load' ported over to 'save'
- we can just remove all the items before binding them from the form (thus no expected support for sparse binding) - form-model can be changed at will
PROPOSED SYNTAX: <fb:repeater id="rep-id" parent-path="." row-path="item" clear-items-on-save="true"> <!-- no fb:identity --> <!--required elements for this case --> <fb:on-bind>...</fb:on-bind> <fb:on-insert>..</fb:on-insert> <fb:on-delete>..</fb:on-delete> </fb:repeater>
EXPECTATION DETECTION: @clear-items-on-save="true"
STRATEGY on 'save': - remove all items using 'on-delete' - do as extended-case-1
REMARK:
This strategy obviously needs to delete and re-create all items. Thus it requires the presence of <fb:insert> and <fb:delete> bindings to operate.
However, taking into account the current operation, and some typing-economics for the most logical/frequent use we should probably agree on some default behaviour for those and request explicit removal of the insert and delete bindings by adding the empty configuration element.
Maybe it's just to late for thinking, but what's the difference between 1 (especially with "straightforward extension") and 2?
[case 3] DESCRIPTION: - items have an explicit identity and can be sparsly bound - form-model can add/remove/reorder the rows - the sequence editing of the rows need to be reflected onto the items
PROPOSED SYNTAX: <fb:repeater id="rep-id" parent-path="." row-path="item" > <!--required elements for this case --> <fb:identity>...</fb:identity> <fb:on-bind>...</fb:on-bind> <fb:on-insert>..</fb:on-insert> <fb:on-delete>..</fb:on-delete> </fb:repeater>
EXPECTATION DETECTION: <fb:identity> is present
STRATEGY on 'save': - backup all items to a list OriginalItems - foreach row in the repeater - if row-identity-match in OriginalItems - move item back into the context and bind - else - use on-insert and on-bind to create and bind - for the items still left in the OriginalItems - use the on-delete
-o0o-
[case 4] (aka current RepeaterBinding)
DESCRIPTION:
- items have an explicit identity and can be sparsly bound
- form-model can add/remove but should not allow reordering of the rows
- the original sequence of the items is to be maintained at all times (on-insert really is more of an on-append: no new ones can be inserted in between the old ones)
PROPOSED SYNTAX: not clear yet, I tend towards:
<fb:repeater id="rep-id" parent-path="." row-path="item" row-path-insert="newItem"> <!--required elements for this case --> <fb:identity>...</fb:identity> <fb:on-bind>...</fb:on-bind> <fb:on-insert>..</fb:on-insert> <fb:on-delete>..</fb:on-delete> </fb:repeater>
EXPECTATION DETECTION:
fb:identity plus
the presense of a @row-path-insert seems to indicate that the new ones need to go into a different space then the exisiting ones, and thus denying the approach expressed in case3
(i.e. even if it would read the same as row-path! so there would be a difference between explicitely stating it as the same, and defaultly assuming it is the same)
STRATEGY on 'save': - foreach row in repeater - if identity match found in items - bind to that - add it to the set of updatedItems - else - add it to some list of rowsToInsert - run through items - if NOT found in updatedItems - add to list of toDeleteItems (ndx will do) - register the on-insert as factory - foreach row in rowsToInsert - create and bind it - foreach ndx in toDeleteItems in reverse order - use on-delete to remove/mark
3 vs. 4: IMO @row-path-insert makes the strategy not really obvious. Wouldn't it be better to specify explicitely whether reordering is allowed/needed or not?
For completion:
implementation of the above assumes the refactoring of the current identity approach towards the earlier mentioned IdentityBinding interface.
see: http://marc.theaimsgroup.com/?l=xml-cocoon-dev&m=107955523607012&w=2
(Joerg, if you're ok, I'll start doing it when this discussion (if any) converges)
Of course, you only remove another lame excuse for starting with my diploma thesis :)
While writing up this proposal I end up questioning a number of (historically chosen) names that could change:
- repeater/@parent-path --> repeater/@path for consistency with the order bindings
Have I written this somewhere above? :-)
- repeater/@row-path --> repeater/@item-path since it is pointing to items, not rows
Indeed, so it's reasonable.
- on-insert is actually more of a on-create ('insert' seems to exclude 'append', while create is more nutral to exactly where the newly created is added, it is also more in sync with the actual factory registration IMHO )
Not that important, but I'm also ok with this.
- repeater/@row-path-insert --> repeater/@new-item-path which would be syncing up with the last two changes and arguments
What do we need @row-path-insert/@new-item-path for if we have on-insert?
Joerg
