ubuntu-touch-coreapps-reviewers team mailing list archive

[Riccardo Padovani <riccardo@xxxxxxxxxxxxx>]

Review: Needs Information

Thanks for working on this :-) 

I left some comments inline - nothing wrong, but I'm not sure about some choices

Diff comments:

> 
> === modified file 'src/app/qml/components/NotebooksDelegate.qml'
> --- src/app/qml/components/NotebooksDelegate.qml	2015-03-04 00:23:45 +0000
> +++ src/app/qml/components/NotebooksDelegate.qml	2015-03-21 04:19:08 +0000
> @@ -133,12 +133,13 @@
>                  horizontalAlignment: Text.AlignRight
>              }
>              Icon {
> -                anchors { left: parent.left; verticalCenter: parent.verticalCenter; right: parent.right }
>                  height: width
> +                width: parent.width
> +
>                  name: "go-next"
>              }
>              Icon {
> -                anchors { left: parent.left; bottom: parent.bottom; right: parent.right }
> +                width: parent.width

Why this change? AFAIK anchors has better performance on resize than width - since we want a convergent application, performance on resize it's important

>                  height: width
>                  name: model.loading ? "sync-updating" : model.syncError ? "sync-error" : model.synced ? "sync-idle" : "sync-offline"
>                  visible: NotesStore.username !== "@local" && (!model.synced || model.syncError || model.loading)
> 
> === added file 'tests/qml/RemindersTestCase.qml'
> --- tests/qml/RemindersTestCase.qml	1970-01-01 00:00:00 +0000
> +++ tests/qml/RemindersTestCase.qml	2015-03-21 04:19:08 +0000
> @@ -0,0 +1,53 @@
> +import QtQuick 2.2
> +import QtTest 1.0
> +import Ubuntu.Components 1.1
> +import Ubuntu.Test 0.1
> +import Evernote 0.1
> +
> +import '../../src/app/qml/'
> +
> +
> +UbuntuTestCase {
> +
> +    // toIndex: 1, 2, 3 for rightActions, -1 for leftAction, defaults to 1
> +    function dragListItemWithAction(delegate, actionIndex) {
> +        if (actionIndex == undefined) {

Isn't better to write `actionIndex = actionIndex || 1;` ?

> +            actionIndex = 1;
> +        }
> +        tryCompare(delegate, "_showActions", false);
> +        waitForRendering(delegate);
> +
> +        var x = delegate.width / 2;
> +        var y = delegate.height / 2;
> +        var dx = units.gu(30) * -actionIndex;
> +        mousePress(delegate, 1, 1);
> +        mouseMoveSlowly(delegate, x, y, dx, 0, 10, 20);
> +        mouseRelease(delegate, x + dx, y);
> +        waitForRendering(delegate);
> +        tryCompare(delegate, "swipping", false);
> +
> +        var action;
> +        if (actionIndex == -1) {
> +            action = findChildWaiting(delegate, "leftAction");
> +        } else {
> +            print("FAIL searching for rightaction","rightAction" + (actionIndex - 1))

What's that fail? 
As far as I understand this branch of the if is executed each time there is a right action...

> +            action = findChildWaiting(delegate, "rightAction" + (actionIndex - 1));
> +        }
> +        print("#### found action", action)
> +        mouseClick(action, 1, 1)
> +        if (actionIndex >= 0) {
> +            tryCompare(delegate, "_showActions", false);
> +        }
> +        waitForRendering(delegate)
> +    }
> +
> +    function findChildWaiting(root, objectName, timeout) {
> +        if (timeout == undefined) {
> +            timeout = 2000;
> +        }
> +
> +        tryCompareFunction(function() {return findChild(root, objectName) != null}, true, timeout);
> +        return findChild(root, objectName);
> +    }
> +}
> +
> 
> === modified file 'tests/qml/tst_NotesPage.qml'
> --- tests/qml/tst_NotesPage.qml	2015-03-21 04:19:08 +0000
> +++ tests/qml/tst_NotesPage.qml	2015-03-21 04:19:08 +0000
> @@ -177,5 +187,71 @@
>                  compare(delegate.title, "testNote" + data.sortOrder[i]);
>              }
>          }
> +
> +        function test_setReminder() {
> +            var note = createNote("testNote1");
> +
> +            var noteDelegate = findChild(mainView, "notesDelegate0");
> +            var reminderIcon = findChild(noteDelegate, "reminderIcon");
> +
> +            // Make sure no reminder set yet.
> +            tryCompare(reminderIcon, "visible", false);
> +
> +            dragListItemWithAction(noteDelegate, 1);
> +
> +            var setReminderView = findChildWaiting(root, "setReminderView");
> +            var saveButton = findChild(setReminderView, "saveButton");
> +
> +            waitForRendering(setReminderView);
> +            mouseClick(saveButton, 1, 1)
> +
> +            tryCompare(reminderIcon, "visible", true);
> +
> +            waitForRendering(mainView)
> +            dragListItemWithAction(noteDelegate, 1);
> +
> +            setReminderView = findChildWaiting(root, "setReminderView");
> +            var deleteButton = findChild(setReminderView, "deleteButton");
> +
> +            waitForRendering(setReminderView);
> +            mouseClick(deleteButton, 1, 1);
> +
> +            tryCompare(reminderIcon, "visible", false)
> +

\n is useless here :-)

> +        }
> +
> +        function test_tag() {
> +            var note = createNote("testNote1");
> +            var noteDelegate = findChild(mainView, "notesDelegate0");
> +
> +            dragListItemWithAction(noteDelegate, 2)
> +
> +            var newTagTextField = findChildWaiting(root, "newTagTextField");
> +            mouseClick(newTagTextField, 1, 1);
> +
> +            typeString("testTag1");
> +
> +            var createTagButton = findChild(root, "createTagButton");
> +            mouseClick(createTagButton, 1, 1);
> +
> +            var closeButton = findChild(root, "tagsDialogCloseButton");
> +            mouseClick(closeButton, 1, 1);
> +
> +            tryCompareFunction(function() {return findChild(root, "createTagButton") == null}, true);
> +
> +            var tagsLabel = findChild(noteDelegate, "tagsLabel");
> +            tryCompare(tagsLabel, "text", "testTag1")
> +
> +            dragListItemWithAction(noteDelegate, 2);
> +
> +
> +            var testTagDelegate = findChildWaiting(root, "tagDelegate0");
> +            mouseClick(testTagDelegate, 1, 1);
> +
> +            closeButton = findChild(root, "tagsDialogCloseButton");
> +            mouseClick(closeButton, 1, 1);
> +
> +            tryCompare(tagsLabel, "text", "")
> +        }
>      }
>  }


-- 
https://code.launchpad.net/~mzanetti/reminders-app/qmltest3/+merge/253753
Your team Ubuntu Reminders app developers is subscribed to branch lp:reminders-app.