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

Revision history for this message
Aurélien Gâteau (agateau) 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.

I think it is because the MenuBarWidget eventFilter is installed as a filter on the menus rather than on the menubar itself (see MenuBarWidget::updateMenuBar()).

> > - 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?

Maybe you could create a separate class which would implement the double-click and drag behavior and would be installed as an event filter on both the menu bar (the QMenuBar, not MenuBarWidget) and the panel?

« Back to merge proposal