[MGNLUI-2995] MultiField shouldn't rely on index/count of the Vaadin child components to define property ids Created: 12/Jun/14 Updated: 16/Jun/14 Resolved: 16/Jun/14 |
|
| Status: | Closed |
| Project: | Magnolia UI |
| Component/s: | forms |
| Affects Version/s: | 5.3 |
| Fix Version/s: | 5.3 |
| Type: | Improvement | Priority: | Major |
| Reporter: | Mikaël Geljić | Assignee: | Mikaël Geljić |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | multifield | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Template: |
|
||||||||
| Acceptance criteria: |
Empty
|
||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||
| Date of First Response: | |||||||||
| Description |
|
MultiField internally maintains its entries as a PropertysetItem, whose entry keys are integers (0-indexed), and whose values are the subfields' properties. Currently, it calculates these integers based on count or index of its child components, which is rather fragile e.g. in case we want to have 'static' components in addition to the variable amount of subfields. In that case, delete removes the wrong property due to mismatch between propertyIds and subfield index within the parent component. We should carefully track what propertyId an entry is created with, and use it for deleting. In case we have to generate a new position manually, size of the itemPropertyIds collection is a better choice over componentCount. |
| Comments |
| Comment by Mikaël Geljić [ 13/Jun/14 ] |
|
https://git.magnolia-cms.com/gitweb/?p=magnolia_ui.git;a=shortlog;h=refs/heads/MGNLUI-2995-mge |
| Comment by Mikaël Geljić [ 16/Jun/14 ] |
|
Relying on propertyId set during initialization was a no go (1st attempt, commit 0a0d31f), because after deleting one entry, itemIds are reorganized, see MultiField#removeValueProperty(int). In order to make sure we always delete the expected property, we fetch it again dynamically during remove (2nd attempt, commit e91a489), see #findPropertyId(Item, Property). See new branch https://git.magnolia-cms.com/gitweb/?p=magnolia_ui.git;a=shortlog;h=refs/heads/MGNLUI-2995-2-mge |