Code review comment for lp:~3v1n0/unity/lim-panel

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

> "The diff has been truncated for viewing. "
>
> Try to avoid doing too much rewriting in any one branch :)

Yes, I know, but things often come in mind when doing it, and as I'm touching a lot of panel stuff, I'd prefer to keep all here.
You can just use bzr send lp:unity -o branch-changes.diff to check them locally ;)

> The empty destructor's definition can be removed (unless its virtual)
>
> + if (!wm->IsExpoActive() && !wm->IsScaleActive())
> 490 + {
>
> This probably doesn't make sense living where it is. The Panel views should
> block all clicks if if the screen is grabbed by compiz period (eg, add a
> wrapper method in WindowManager around CompScreen::otherGrabExist (NULL);)

Well, if the dash grab isn't considered by that, I could try to do it.

> 3391 + // FIXME: compiz doesn't give us a valid xid (is always 0 on unmap)
> 3392 + // we need to do this again on BamfView closed signal.
>
> Great. I'll fix that in compiz then. (This is probably the case when the
> window is destroyed and the xid is changed to zero, and then the unmap signal
> happens)

Let me know when your fix will be landed, so we can remove this.

> The window management stuff looks great.

Cool! ;)

« Back to merge proposal