Code review comment for lp:~dyams/unity-2d/altf1-toggle-launcher-focus

Revision history for this message
Lohith D Shivamurthy (dyams) wrote :

> I see no need to add /* lp:885304 */ as a note in the source code. The
> function name makes it obvious what it's doing
>
> Some style corrections: instead of
>
> if (!main.activeFocus)
> main.focus = true
>
> please use either
>
> if (!main.activeFocus) main.focus = true
>
> or
>
> if (!main.activeFocus) {
> main.focus = true
> }
>
>
> Also please consult the CODING document for the use of braces with if/else
> blocks in C++, this is wrong:
>
> }
> else {

Sorry.

I have updated the branch with above changes now.

>
>
> Would you have any comment on why Alt+F1 isn't working while you are in the
> Spread?

Yes, I have. Currently, when Spread is will eat all mouse and keyboard events while it is open.
IMO, We need a separate discussion for this.

« Back to merge proposal