> On 2009-10-06 10:16:00, Jacopo De Simoi wrote: > > /trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.cpp, > > line 390 > > <http://reviewboard.kde.org/r/1790/diff/3/?file=12370#file12370line390> > > > > Now that the left(hehe)action stays activated when !isCollapsed() we > > should call m_leftActionIcon->setVisible(m_hovered) here > > > > wrote: > the real problem here is that mouse release off of the item is accepted; > it shouldn't be possible to collapse with a mouse click and not still be > hovered. fixed in my local copy.
indeed it /is/ possible to collapse one item simply by expanding another item, without touching the former. > On 2009-10-06 10:16:00, Jacopo De Simoi wrote: > > /trunk/kdereview/plasma/applets/devicenotifier-refactor/notifierdialog.cpp, > > line 484 > > <http://reviewboard.kde.org/r/1790/diff/3/?file=12372#file12372line484> > > > > 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) > > > > wrote: > item->setHovered(false) is called on HoverLeave in > NotifierDialog::eventFilter. the timer is not responsible for setting an item > as not hovered, only for setting it as hovered. > > as for the timer, m_clearItemBackgroundTargetTimer.start() is called when > any device item gets a HoverLeave, collapsed or selected. it is also called > when a selected item gets a HoverEnter. so it is always started except when a > non-collapsed (aka selected) item gets a HoverEnter. i don't see where this > timer could get out of step of the events. > > so i don't see a problem in either case. The problem is the following: suppose you have 2 items: x and y; all times are expressed in ms: at time 0 you leave item x at time 50 you hover on item y, m_itemBackground starts its animation from item x to item y at time 300 you leave item y and you do not hover any other item (e.g. out of the plasmoid), the timer is started; y->setHovered(false) at time 350 m_itemBackground arrives on y, signals targetReached, which calls itemHovered, which calls y->setHovered(true) at time 400 m_itemBackground gets hidden, but nobody resets the status of item y, which stays in the "hovered" state. Despite the fact that is seems complicated to reproduce, just waving your mouse over the plasmoid triggers this situation > On 2009-10-06 10:16:00, Jacopo De Simoi wrote: > > /trunk/kdereview/plasma/applets/devicenotifier-refactor/deviceitem.cpp, > > line 152 > > <http://reviewboard.kde.org/r/1790/diff/3/?file=12370#file12370line152> > > > > Here we should make sure to hide() the description Label; > > wrote: > playing around with it some more, it feels odd to have some parts > disappearing and others staying ... i've made all the header items remain > when the item is expanded. the downside to this is that the capacity bar > won't update when the item is expanded. As for making the description label less redundant, if we really feel bad about it, we could add a ":" at the end when the device gets expanded... However, the capacity bar should /always/ display a proper value, I understand that it would get updated on hover anyways, but that seems to me more broken than hiding/showing it. Perhaps we should show it in a less glamorous way, for instance as a tiny capacity bar below the icon (a few pixels high) and the label (xxx Gib free) below the description... then seeing it appear and disappear should bother the user that much... Mmh, that doesn't even convince me. - Jacopo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1790/#review2562 ----------------------------------------------------------- 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 > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
