Review Request: Fix Chat Plasmoid Scrolling Bug (293522)

[David Edmundson]

> On July 12, 2012, 8:31 a.m., David Edmundson wrote:
> > plasmoid/org.kde.ktp-chatplasmoid/contents/ui/ChatWidget.qml, line 142
> > <http://git.reviewboard.kde.org/r/105521/diff/1/?file=72093#file72093line142>
> >
> >     why 9001?
> 
> Lasath Fernando wrote:
>     Needed to be a large number that's constant - First one I could think of. 
>     
>     I don't think it really matters how big it is, as long as it's higher than the amount of messages there are.

does it need to be higher than the number of messages, or just any quite big number? I imagine the average delegate size over the last 40 or so will probably be reasonably accurate enough, but I've not experimented with it.

I don't want to completely maul a user's computer. 


> On July 12, 2012, 8:31 a.m., David Edmundson wrote:
> > plasmoid/org.kde.ktp-chatplasmoid/contents/ui/ChatWidget.qml, line 157
> > <http://git.reviewboard.kde.org/r/105521/diff/1/?file=72093#file72093line157>
> >
> >     what does this do?
> 
> Lasath Fernando wrote:
>     Makes it scrollable. It's on by default so it doesn't really do anything. I added that while trying to figure out what was different between this and the ListView in the ContactList plasmoid.
>     
>     Should I remove it?

I'd not seen it placed on the scrollbar itself before. Leave it.


> On July 12, 2012, 8:31 a.m., David Edmundson wrote:
> > tests/message-processor-basic-tests.h, line 51
> > <http://git.reviewboard.kde.org/r/105521/diff/1/?file=72095#file72095line51>
> >
> >     This isn't part of the same patch.
> 
> Lasath Fernando wrote:
>     Uh oh... It seems to have a commit from another branch. How can I fix this without cherry-picking?

if it's in another commit, that's fine.. interactive rebase FTW. ping me if you need a hand.


- David


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


On July 12, 2012, 7:54 a.m., Lasath Fernando wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105521/
> -----------------------------------------------------------
> 
> (Updated July 12, 2012, 7:54 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> This fixes the annoying bug stopping the plasmoid from scrolling properly as elegantly as possible.
> 
> The problems is that because delegates are created on demand, there's no way for the ListView to determine the height of all its content. It currently just takes the size of one delegate, and multiplies it by the amount of items in the model. While that works in scenarios where the delegates are all the same height, it does not here.
> 
> What this does is -abuse- use the cacheBuffer property to force it to keep all delegates in ram. While it uses more memory, it let's us work around this issue (Only other option is to remove the scrollbar). 
> 
> PS: I tried to set the cacheBuffer to view.count, but that made it ignore it completely (I think it can't change after initialized for it to be taken seriously).
> 
> 
> This addresses bug 293522.
>     http://bugs.kde.org/show_bug.cgi?id=293522
> 
> 
> Diffs
> -----
> 
>   plasmoid/org.kde.ktp-chatplasmoid/contents/ui/ChatWidget.qml 90cc07d 
>   plasmoid/org.kde.ktp-chatplasmoid/contents/ui/OutgoingDelegate.qml d713958 
>   tests/message-processor-basic-tests.h 35f931f 
>   tests/message-processor-basic-tests.cpp 554ce1a 
> 
> Diff: http://git.reviewboard.kde.org/r/105521/diff/
> 
> 
> Testing
> -------
> 
> Talked to myself.
> 
> 
> Thanks,
> 
> Lasath Fernando
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120712/c8ca7252/attachment.html>