KCapacitybar on places item view

[Kevin Ottens]

Le Monday 04 August 2008, Rafael Fernández López a écrit :
> - Show capacity bar only on hover, or on selection.
>
> - Poll only devices that are showing the capacity bar (that means, if
> selected or hovered).

Although I would have preferred the tooltip solution. Doing it like this is 
much better visually. The widget won't get overcrowded, and the capacity bar 
only appears where your attention is (selection or hover).

Also, it'll generate much less polling which is good. Although that's 
debatable because of the selection (much longer term polling).

> - Avoid clearing the selection by clicking (or double clicking) on the
> viewporrt (invalid index). Why I implemented it into the event filter ?
> Reimplementing our own QItemSelectionModel wouldn't work since we do clear
> the selection from our own code (when we write for example an address that
> doesn't match any entry), and that case would be stopped too. This means
> that the only thing we want to do is to stop the user clearing the
> selection by clicking directly on the viewport.

Fine with me.

> Suggestions:
>
> - Add a context menu option for (show always capacity bar for removable
> devices). Should I do this ?

Honestly I'd avoid it. It makes an option for this widget in the context menu, 
then you'd have to handle the case where it's for Dolphin only or also for the 
file dialog. It'll be attached to the context menu for all items, even if that 
doesn't make sense. Also would reopen the "too much polling" case of course...

Sounds like a bad idea, and really it's not as if the information is not at 
hand with the hovering and the selection.

Now my comments on the patch itself.

1) It's unfortunately a no-go in its current form as I tested it and got the 
attached screenshot. Note really well placed I think. ;-)

2) Please don't move the KFilePlacesView::Private class inside the cpp file. 
The current layout is perfectly intended, I'd like to avoid coupling the 
delegate with the view class as much as possible. The idea being that at one 
point we could make the delegate more generic and help get the kind of 
animations available in the places delegate to other places. The coupling is 
already bad as it is, this move and the friend directive makes it worse. 
++beer from me if you avoid it.

3) _k_timeLineValueChanged() should probably have another name. It's not the 
only time line used in there, so please make it more specific.

4) _k_slotPollDevices() should be renamed too. We don't use the "slot" 
convention in names for this class.

5) The indentation in KFilePlacesEventWatcher event filter should be modified 
to avoid the two aligned } at the end of each case block.

Other than that it seems fine style wise.

Regards.
-- 
Kévin 'ervin' Ottens, http://ervin.ipsquad.net
"Ni le maître sans disciple, Ni le disciple sans maître,
Ne font reculer l'ignorance."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: places-capacitybar-breakage.png
Type: image/png
Size: 22445 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080805/5a6447bb/attachment.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080805/5a6447bb/attachment.sig>