Code review comment for lp://staging/~saviq/unity-2d/unmaximize-on-drag

Revision history for this message
MichaƂ Sawicz (saviq) wrote :

> - The code you added to ClosedMenuBarHelper should be in MenuBarWidget: I
> would like ClosedMenuBarHelper to remain a small helper class to notice when a
> menu in the menubar is closed.

For some reason I didn't get the events in MenuBarWidget, but now you mentioned it they obviously should happen there. Maybe that's even the cause of the Hover events getting lost.

> - Prefer using "const QPoint&" instead of "QPoint*" in WindowHelper::drag()
> and in MenuBarWidget::menuBarDragged(). Using QPoint* makes it look like the
> code is going to modify the point.
>
> - Prefer using a forward declaration of QPoint class in windowhelper.h instead
> of #including it.

Noted.

> - I am worried with r564: it looks like a duplication of the code you added in
> MenuBarWidget. Maybe it is possible to do most of the work from MenuBarApplet?

Yes it definitely is duplication that I disliked, too. IMO there should simply be a 'QDragEvent' available that does the heavy lifting. Since the widgets alternate in visibility, what approach do you propose to get rid of that?

« Back to merge proposal