[MGNLUI-3554] Thumbnail View in magnolia 5.4 doesn't react anymore to changes in the underlining datasource Created: 05/Sep/15 Updated: 15/Apr/16 Resolved: 06/Oct/15 |
|
| Status: | Closed |
| Project: | Magnolia UI |
| Component/s: | workbench |
| Affects Version/s: | 5.4, 5.4.1 |
| Fix Version/s: | 5.4.3 |
| Type: | Bug | Priority: | Critical |
| Reporter: | Fabrizio Giustina | Assignee: | Aleksandr Pchelintcev |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | regression | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||
| Issue Links: |
|
||||||||
| Template: |
|
||||||||
| Patch included: |
Yes
|
||||||||
| 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)
|
||||||||
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
||||||||
| Date of First Response: | |||||||||
| Sprint: | Basel 14 | ||||||||
| Story Points: | 5 | ||||||||
| Description |
|
Since Magnolia 5.4 (after 5.3 m10) the thumbnail layout is not refreshed properly anymore when the datasource is changed. The change in This change totally broke our UIs based on the previous ThumbnailContainer, which should have been compatible with the new implementation. The worst thing is that at the moment it seems there is no way to force a refresh, even calling refresh() in LazyThumbnailLayout or clearing the cache in LazyThumbnailLayoutConnector is not useful at all. For example if you use the thumbnail container to display the content of a specific folder, updating the datasource (selecting a different folder) without destroying/recreating the panel this is what happens:
If not updating automatically there should be at least a way to force a real refresh of the content, however I am not sure what kind of event/notification would be better to use here. As a partial patch we fixed it by reinitializing the panel when the "thumbnailAmount" property changes in LazyThumbnailLayoutConnector, however this is not perfect because it only fixes 1) and 2) but doesn't help when the datasource is refreshed without changing the total number of items. This is really a blocker for us for properly upgrading to magnolia 5.4, and also patching the widgeset is a real pain... It would be great to have at least the attached patch included for magnolia 5.4.2, and if somebody could review the LazyThumbnailLayout implementation in order to properly react even if the number of items is not changed. |
| Comments |
| Comment by Fabrizio Giustina [ 05/Sep/15 ] |
|
another side note connected to the changes in thumbnail layout/container with magnolia 5.4: ThumbnailContainer has been deprecated and javadocs say it should be replaced by "info.magnolia.ui.workbench.thumbnail.data.DelegatingThumbnailContainer", but this class doesn't exist and I could't find any actual replacement... should it not have been deprecated or am I missing something?
/**
* @deprecated since 5.3.10 in favor of {@link info.magnolia.ui.workbench.thumbnail.data.DelegatingThumbnailContainer}, this container
* should be avoided as it loads the items eagerly.
*/
public class ThumbnailContainer
|
| Comment by Aleksandr Pchelintcev [ 01/Oct/15 ] |
|
fgiust Thanks a lot for spotting the issue and providing such an in-depth analysis, we'll try to fix this one asap. However, in order to understand it better - could you pls elaborate a bit on how exactly you change the underlying data-source dynamically? Btw info.magnolia.ui.workbench.thumbnail.data.DelegatingThumbnailContainer indeed doesn't exist - it used to but during refactroring it was renamed to info.magnolia.ui.workbench.thumbnail.JcrThumbnailContainer which backs the thumbnail view at the moment. JcrThumbnailContainer is almost the same container that is used for the list view now and is supposed to be more performant since it doesn't resolve all the thumbnail urls on initialisation (unlike the old one). |
| Comment by Fabrizio Giustina [ 12/Oct/15 ] |
|
Hi @apchelintcev,
Well, the use case here is having a tree which controls the thumbnail view. When switching from a folder to another the content is changed without recreating the thumbnail view. ... extends AbstractContainer
implements Refreshable, info.magnolia.ui.vaadin.layout.data.ThumbnailContainer
public void setFolder(String workbenchPath)
{
this.workbenchPath = workbenchPath;
...
}
public Collection< ? > getItemIds()
{
// query the current folder, so items could change after the setFolder() call
...
}
public void refresh()
{
refreshTotalSize(); // recalculates the total size, which triggers the refresh with the patch described above
fireItemSetChange(); // ignored at the moment by LazyThumbnailLayoutConnector
}
Let me know if you need any additional detail, I would be glad to help or test any proposed fix. |
| Comment by Aleksandr Pchelintcev [ 13/Oct/15 ] |
|
fgiust thanks for this. However, I managed to reproduce the issue myself with some quick and dirty solution that logically resembles yours. The branch fix/ |
| Comment by Fabrizio Giustina [ 13/Oct/15 ] |
|
great news, I'll try the fix asap, thanks Aleksandr! |