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

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

Good... Some comments:

124 + monitor_geo.x += XY_OFFSET;
125 + monitor_geo.y += XY_OFFSET;
126 + monitor_geo.width -= WH_OFFSET;
127 + monitor_geo.height -= WH_OFFSET;

What about monitor_geo.OffsetSize() and monitor_geo.OffsetPosition() ?

139 + view_->hide_request.connect ([this] (bool activate) { Hide(activate); });

Since both have a bool as parameter, you can just use sigc::mem_fun or std::bind there...

994
  LayoutWindow::Ptr layout_window(0);

No need to set it to 0, however you can also just return inside the loop and return nullptr otherwise, g++ will be smart enough to convert.

UnityScreen::get (screen)

Get rid of the spaces

825 + LayoutWindow::Ptr const &layout_window = UnityScreen::get(screen)->
826 + GetSwitcherDetailLayoutWindow(window->id());

One line please (in case use auto const&).

Some code in lines 659-673 and 681-696 (middle click mouse handling in UnityWindow) is duplicated, what about adding a function to check if the position hits a window thumbnail (that basically checks the case of the switcher and of the scale).

I've also incurred into a regression: if I select the top-left window in detail mode (or the only one existent in case we have just one), then the switcher selects the window but it doesn't hide.

Finally, this not happen always, but when I do click -> move the mouse outside the clicked area -> release, the switcher closes

« Back to merge proposal