[Panel-devel] Plasma::Icon highlighting issues

[Jason Stubbs]

On Friday 14 December 2007 01:40:33 Aaron J. Seigo wrote:
> the ButtonState -> Interaction flag changes are a bit gratuitous
> and i'm not overly fond of the new names for the flags nor the 
> lack of a named state for no state / interaction.

After looking up "gratuitous" in a dictionary, I guess you mean "Unnecessary 
or unwarranted; unjustified"? I changed it to this way because the relevant 
combinations are (using the patch's terminology):

- 0 (regular icon)
- MouseHovering (-hover icon + hover effect)
- ButtonPressed | MouseHovering (-pressed icon)
- ButtonPressed (regular icon)
- AwaitingRelease (-pressed icon)

The names NoState, HoverState and PressedState are mapping directly on to the 
icon's appearance whereas the mouse button's state needs to be monitored 
separately even without the AwaitingRelease addition. I also don't think 
ButtonState of HoverState is really correct seeing that mouse's button isn't 
really hovered - unless ButtonState is really IconState?

Anyway, I don't really care what names they are as long as the above 5 states 
can be represented. Supply me some names and I'll use them instead. :)

> minor style nitpick:
>
> +    } else if (!interactions.testFlag(ButtonPressed)
> +        && interactions.testFlag(MouseHovering)) {
>
> should be:
>
> +    } else if (!interactions.testFlag(ButtonPressed) &&
> +                   interactions.testFlag(MouseHovering)) {

Ok.

> > * Remove the pressed(bool) signal as kickoff is the only code using it
> > and the next patch changes that
>
> it's there for completeness. think of menus that use this button. it needs
> to stay.

Is pressed(false) ever relevant? I'd at least like to remove that.

> > (and I think that actions being triggered before a release on what is 
> > essentially a button widget is a Bad Thing(TM))
>
> when the button causes an action to start, yes. when it results in a popup
> such as kickoff, it isn't.

Yep, acknowledged in my response to Marco last night.

> > * Set the icon's delayedRelease property so that the icon appears to stay
> >   pressed while the menu is visible
>
> cool.
>
> > * Return the icon to normal behaviour after the menu disappears
>
> cool. there's a BR somewhere for this as well in the plasma component.

That's why I'm working on it. The war with bugzilla can never be won, but 
lower bug numbers makes things easier for everybody. :)

On Friday 14 December 2007 01:57:21 Aaron J. Seigo wrote:
> i'm also not completely happy about this:
>
> // Also remove the MouseHovering flag even though the mouse may still be in
> // the icon's area as the hoverLeaveEvent is sometimes not received. After
> a // click, it looks more natural to have an unhovered icon when the mouse
> is // over it than to have a hovered icon when the mouse isn't over it.
>
> since that certainly does ensure it isn't hovered, but it also makes it
> look rather broken after being clicked, particularly with button svg's that
> have hover backgrounds. =/

Does "clicked" mean "clicked()" or "pressed()"? If you mean the appearance of 
the icon after the kickoff menu appears, that's intentional and is not 
affected by the above - see the states I listed at the top of this mail.

The intention of the above is only to prevent the icon from appearing hovered 
after the kickoff menu closes, although I think my second patch last night 
might have broken behaviour after a clicked() slightly as well.

> this is a problem we have elsewhere as well, e.g. the applet handles.
> hacking around it randomly in various classes seems pretty lame. we need
> this addressed elsewhere, e.g. QGraphicsScene.

There's another hack in there too. ;)

  // Due to an issue with QGraphicsView's mouse grabbing, mouseReleaseEvent()
  // isn't called if the mouse leaves the QGraphicsView. If there is a
  // hoverEnterEvent, it means that the drag is no longer happening so clear
  // the ButtonPressed flag to ensure everything is correct.
  d->interactions &= ~Private::ButtonPressed;

The only bad behaviour without the hover-reset hack is that the icon will 
sometimes appear as if it's being hovered over when it isn't. The hack that 
clears ButtonPressed causes no visible difference in current usages of 
Plasma::Icon, though it could cause some issues in corner-cases when 
IconActions are also in use. If that's fine with you (until Qt is fixed), 
then it's fine with me too.

-- 
Jason Stubbs