Code review comment for lp:~unity-team/unity8/mirCompositor

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

=== added file 'qml/Stages/TabletStage.qml'
+ objectName: "stages"
that a good name? Also PhoneStage has no objectName set at all, should we add one?

"overlay" = sidestage effectively, right? Just the term threw me for a bit. Rename isn't needed, but perhaps a comment saying so would be nice.

"invalid" state is when no app is open, so spread should not be shown? That really an "invalid" state, or just empty?

+ property int phase
You were kind enough to add a nice comment in PhoneStage explaining the different stages, could you do the same here please?

+ property bool locked: spreadView.phase == 2
+ property string focusedAppId: ApplicationManager.focusedApplicationId
readonly? there's a bunch of properties in the Flickable that could also be readonly.

+ // Add 1 pixel to make sure we definitely hit positionMarker4 even with rounding errors of the animation.
+ snapAnimation.targetContentX = spreadView.width * spreadView.positionMarker4 + 1;
Math.ceil?

"ApplicationInfoInterface.MainStage" <- bit of a mouthful, let's try to shorten that in future :)

+ function indexToZIndex(index) {
a few more comments in here would help, I'm struggling to figure it out.

+ anchors.leftMargin: spreadView.width - spreadView.sideStageWidth + spreadView.sideStageWidth * sideStageDragHandle.progress
can compact the maths a tiny bit

+ } else if (progress < spreadView.positionMarker1 + snappingCurve.period){
nitpick: space before opening brace

« Back to merge proposal