Code review comment for lp:~mzanetti/unity8/right-edge-2

Revision history for this message
Gerry Boland (gerboland) wrote :

+ function hide() {
+ if (locked) {
+ return;
+ }
+ forceHide();
+ }
Why does hide not do anything if the spreadView is visible? Well more like, it sounds like an error that hide is called when spreadView is visible.

+ blockInput: ApplicationManager.focusedApplicationId.length === 0
Nitpick: checking length of the appId to be zero to prove there's no focused app? Not wrong, just reads a bit funny. Maybe String::isEmpty() works in QML, never tried it.

+ property string focusedAppId: ApplicationManager.focusedApplicationId
+ property var focusedApplication: ApplicationManager.findApplication(focusedAppId)
Just checking: the extra focusedAppId property needed to properly update the focusedApplication property? (you do this in a few places, so just curious why)

+++ qml/Stages/PhoneStage.qml
+ onMovingChanged: {
...
+ } else {
+ mainScreenshotImage.visible = false;
Maybe setting mainScreenshotImage.src = "" might free a little memory when spread not open? Probably more GPU memory than system memory. It might have cost too though, so just an idea

+ Image {
+ id: mainScreenshotImage
+ property string src
+ source: src
why not just use source everywhere?

« Back to merge proposal