getavalon / core

The safe post-production pipeline - https://getavalon.github.io/2.0
MIT License
218 stars 49 forks source link

Fix OptionalMenu highlighting in loader #518

Closed iLLiCiTiT closed 4 years ago

iLLiCiTiT commented 4 years ago

Issue to solve:

Suggestion:

davidlatwe commented 4 years ago

Sorry for late reply, was on vacation (Lunar New Year 🎈).

  • OptionMenu sometimes doesn't get mouseMoveEvent and is ignoring mouseEnterEvent, hence it the highlight of actions isn't triggered sometimes.

I haven't seen this before, is this issue happens on specific DCC ?

  • mouse release triggers action where cursor currently is rather than the on where it originally clicked

Hmmm, the behavior was referenced from Maya's menu, and the rule that I applied was "trigger what's been highlighted". Shouldn't it triggers the thing that is currently on top of ?

iLLiCiTiT commented 4 years ago

I haven't seen this before, is this issue happens on specific DCC ?

Each host has same issue, at least: Maya, Nuke. image

Hmmm, the behavior was referenced from Maya's menu, and the rule that I applied was "trigger what's been highlighted". Shouldn't it triggers the thing that is currently on top of ?

Didn't realize but you have got true, it is default at all apps. Maybe it was more user friendly in my point of view. Can be easily changed back...

davidlatwe commented 4 years ago

Ah, for the first issue, I think I've get what you're saying. The mouse movement looks like pretty laggy, see below.

before

But the solution in this PR (removing OptionMenu) will break the highlighting after mouse is pressed and holded, see the comment in #482 👉 here.

Having that said, I have had take a deep look into this and I found the cause was action widget's label did not set mouseTracking to True ! And that's why it appears laggy, since there's a big mouse movement dead zone (the rect of label widget) in each action widget !

So just adding label.setMouseTracking(True) into OptionalActionWidget.__init__ would solve the issue, see below.

after

🎉

tokejepsen commented 4 years ago

Looks promising @davidlatwe !

davidlatwe commented 4 years ago

Thanks @tokejepsen , see if @iLLiCiTiT could confirm what I have found. :)

iLLiCiTiT commented 4 years ago

see if @iLLiCiTiT could confirm what I have found. :)

@davidlatwe I can confirm it works :)

But to be hones I don't like that mouse event (highlighting) is handled in parent item instead of item itself, because OptionalAction is not usable in QMenu (which may be handy in future), but the true is, nobody will touch this code for a long time so I believe it doesn't matter.

...will break the highlighting after mouse is pressed and holded ...

Qt is fully customizable if you know how :) (I don't know at this moment but it won't be a problem to try)

Now question is if I should cancel this PR and create new (or you'll create new)?

davidlatwe commented 4 years ago

But to be hones I don't like that mouse event (highlighting) is handled in parent item instead of item itself

I don't like it, too ! But it was painful to find the way to poperly implement the desired behavior from each item. 🤕

Qt is fully customizable if you know how :)

Agree. 🛠

Now question is if I should cancel this PR and create new (or you'll create new)?

I think I would close this one and reference it in the new PR. And would be great if you could submit a new one. :)

iLLiCiTiT commented 4 years ago

Ok, I let this PR open for now, to keep it in my mind, and will look if is possible to implement mouse events in OptionalActionWidget with same mouse press/release events like now. If I won't find it today I'll create new PR this evening with your solution.

iLLiCiTiT commented 4 years ago

I had a little bit of free time for exploration and result is that it is possible, but only with overriding QMenu. Reason is that QMenu by default send mouse move events only to pressed action. So to be able change highlight of hovered actions after mouse press it is necessary to override QMenu to trigger mouse move events for all actions. I would prefer to override QMenu and highlighting changes let for action widget but that is time consuming stuff so I'll create PR with your solution :)

With this change:

iLLiCiTiT commented 4 years ago

moved to #521