Review Request 111632: Fix Bug 302703 - [Dolphin 2.1] Icons view mode 'messed up' after switching from details view mode (w/o expandable folders)

[Frank Reininghaus]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111632/#review36287
-----------------------------------------------------------


Thanks for the analyzing this problem.

Hm, I looked at the code, and I have the impression that your patch hides the real bug (and also adds some unnecessary overhead every time KItemListView::doLayout() is called).

Actually,

KStandardItemListView::onItemLayoutChanged() already calls KStandardItemListView::updateLayoutOfVisibleItems(), which then calls initializeItemListWidget().

However, DolphinItemListView::onItemLayoutChanged(), which overrides KFileItemListView::onItemLayoutChanged(), does not call the base class implementation because the answer to the question "do we support expanding or not" is different in both classes - DolphinItemListView takes the "Expandable folders" setting into account.

The code around changing the item layout and the "supports expanding" state is partly duplicated and not quite consistent in KStandardItemListView/KFileItemListView/DophinItemListView. I think we can make it more robust by refactoring it a bit, setting the "supports item expanding" property in KStandardItemListView, and answering the question if the layout supports it in a virtual function. Then we can just do everything that is needed if the state of that property chanes in onSupportsItemExpandingChanged(), and safely make DolphinItemListView::onItemLayoutChanged() call the base class implementation (which fixes the bug):

http://paste.kde.org/p9a70554b/

What do you think?


- Frank Reininghaus


On July 21, 2013, 2:33 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111632/
> -----------------------------------------------------------
> 
> (Updated July 21, 2013, 2:33 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> The view mode for already visible items was not updated while layouting, so call initializeItemListWidget to tell the widget the changed view mode.
> 
> 
> This addresses bug 302703.
>     http://bugs.kde.org/show_bug.cgi?id=302703
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kitemlistview.cpp d2b3fa1 
> 
> Diff: http://git.reviewboard.kde.org/r/111632/diff/
> 
> 
> Testing
> -------
> 
> Works for me.
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130722/c67e1e59/attachment.htm>