Review Request: Unified ItemBackground in new Device Notifier

[Jacopo De Simoi]

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

Ship it!


I'm finally convinced, we need to fix some issues here and there, but we can do it after the commit


/trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.cpp
<http://reviewboard.kde.org/r/1790/#comment1884>

    Here we should now make sure to setVisible(m_hovered) the descriptionLabel



/trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.cpp
<http://reviewboard.kde.org/r/1790/#comment1883>

    Here we should make sure to hide() the description Label; 



/trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.cpp
<http://reviewboard.kde.org/r/1790/#comment1878>

    shouldn't this be (m_hovered || !isCollapsed()) as well?



/trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.cpp
<http://reviewboard.kde.org/r/1790/#comment1881>

    Now that the left(hehe)action stays activated when !isCollapsed() we should call m_leftActionIcon->setVisible(m_hovered) here
    



/trunk/kdereview/plasma/applets/devicenotifier-refactor/notifierdialog.cpp
<http://reviewboard.kde.org/r/1790/#comment1882>

    We have a problem here: 
    itemHovered is connected to targetReached, however since now we have a timer (100ms) to hide the itemBackground it happens that a user could hoverLeave the selected item right before this signal is triggered; which leaves the item in the hovered state.
    
    I suggest adding a m_hoveredItem which is set/reset in eventFilter and is "promoted" to m_selectedDevice here.
    itemHovered should be called only if (item == m_hoveredItem)
       


- Jacopo


On 2009-10-05 23:27:10, Aaron Seigo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1790/
> -----------------------------------------------------------
> 
> (Updated 2009-10-05 23:27:10)
> 
> 
> Review request for Plasma, Jacopo De Simoi and Giulio Camuffo.
> 
> 
> Summary
> -------
> 
> proof of concept showing how the item background could be unified into NotifierDialog; requires today's libplasma for fixes to the ItemBackground widget
> 
> 
> Diffs
> -----
> 
>   /trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.h 1031762 
>   /trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.cpp 1031762 
>   /trunk/kdereview/plasma/applets/devicenotifier-refactor/notifierdialog.h 1031762 
>   /trunk/kdereview/plasma/applets/devicenotifier-refactor/notifierdialog.cpp 1031762 
> 
> Diff: http://reviewboard.kde.org/r/1790/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Aaron
> 
>