Code review comment for lp:~brandontschaefer/unity/alt+tab-mouse

Revision history for this message
Marco Trevisan (TreviƱo) (3v1n0) wrote :

I like a lot the change and the cleanup in event handling for the switcher we had. Even more that we can reuse some code we wrote to get things work easily.
Some comments.

It would be nice if you could abastract a little these signal names:

right_clicked_icon (detail request?)
mouse_moving_over_icon (mouse_move?)

131 + introspection_results_.clear();

Do we really need to keep them around? Why not just a local var?

Anyway:
135 + for (auto target : render_targets_)

322 + int half_size = icon_size.Get() / 2 + 10;

10? Magic number? :)

use auto const& please.

Also there are other behaviors to fix:
* The logic of the scroll wheel, as now here it's inverted to the one we had before.
* The click activation, as it should make sure that you activate an icon only if the mouse-up happened where mouse down happened before.
* The detail selection doesn't work on secondary monitors.
* It would be nice to remove the spread offset (or at least to define it in the controller, just once)

review: Needs Fixing

« Back to merge proposal