ubuntu-touch-coreapps-reviewers team mailing list archive

[Michael Zanetti <michael.zanetti@xxxxxxxxxxxxx>]

thanks! Inline replies

Diff comments:

> === modified file 'src/app/qml/components/EditTagsDialog.qml'
> --- src/app/qml/components/EditTagsDialog.qml	2015-02-27 22:43:12 +0000
> +++ src/app/qml/components/EditTagsDialog.qml	2015-06-11 19:39:25 +0000
> @@ -26,6 +26,12 @@
>  Dialog {
>      id: root
>  
> +    title: i18n.tr("Edit tags")
> +    text: tags.count == 0 ? noTagsText : haveTagsText
> +
> +    property string noTagsText: i18n.tr("Enter a tag name to attach it to the note.")
> +    property string haveTagsText: i18n.tr("Enter a tag name or select one from the list to attach it to the note.")
> +
>      property var note
>      property int pageHeight
>  
> @@ -35,42 +41,91 @@
>          id: tags
>      }
>  
> +    SortFilterModel {
> +        id: tagsSortFilterModel
> +        model: tags
> +        filter.property: "name"
> +        filter.pattern: RegExp(textField.text)
> +    }
> +
>      RowLayout {
>          Layout.preferredWidth: parent.width - units.gu(2)
>          Layout.alignment: Qt.AlignHCenter
> +        z: 2
>  
> -        TextField {
> -            id: textField
> +        Item {
>              Layout.fillWidth: true
> -            placeholderText: i18n.tr("Create a new tag")
> -
> -            onAccepted: accept();
> -
> -            function accept() {
> -                var tagName = text;
> -                text = '';
> -
> -                // Check if the tag exists
> -                for (var i=0; i < tags.count; i++) {
> -                    var tag = tags.tag(i);
> -                    if (tag.name == tagName) {
> -                        // The tag exists, check if is already selected: if it is,
> -                        // do nothing, otherwise add to tags of the note
> -                        if (note.tagGuids.indexOf(tag.guid) === -1) {
> -                            note.tagGuids.push(tag.guid);
> -                        }
> -                        return;
> -                    }
> -                }
> -
> -                var newTag = NotesStore.createTag(tagName);
> -                print("tag created", newTag.name, "appending to note");
> -                note.tagGuids.push(newTag.guid)
> +            Layout.preferredHeight: okButton.height
> +
> +            TextField {
> +                id: textField
> +                placeholderText: i18n.tr("Tag name")
> +                anchors.fill: parent
> +
> +                onAccepted: accept();
> +
> +                function accept() {
> +                    var tagName = text;
> +                    text = '';
> +
> +                    // Check if the tag exists
> +                    for (var i=0; i < tags.count; i++) {
> +                        var tag = tags.tag(i);
> +                        if (tag.name == tagName) {
> +                            // The tag exists, check if is already selected: if it is,
> +                            // do nothing, otherwise add to tags of the note
> +                            if (note.tagGuids.indexOf(tag.guid) === -1) {
> +                                note.tagGuids.push(tag.guid);
> +                            }
> +                            return;
> +                        }
> +                    }
> +
> +                    var newTag = NotesStore.createTag(tagName);
> +                    print("tag created", newTag.name, "appending to note");
> +                    note.tagGuids.push(newTag.guid)
> +                }
> +
> +            }
> +
> +            Rectangle {

Hmm... I tried with a Popover at first, but in this context it didn't feel right. Just having an UbuntuShape without the arrow pointing to the input seems odd too. IMO the rectangle fits best here but not sure... Maybe we can ask popey for a thirst opinion.

> +                x: textField.x
> +                y: textField.y + textField.height

had some issues at first because this thing is outside of the layout. But seems to work fine with anchors after all. fixed.

> +                width: textField.width
> +                color: "white"
> +                border.width: units.dp(1)
> +                border.color: "black"
> +                height: Math.min(5, tagsListView.count) * units.gu(4)
> +                visible: textField.text.length > 0

very good catch! fixed

> +
> +                ListView {
> +                    id: tagsListView
> +                    anchors.fill: parent
> +                    model: tagsSortFilterModel
> +                    clip: true
> +
> +                    delegate: Empty {
> +                        height: units.gu(4)
> +                        Label {
> +                            anchors.fill: parent
> +                            anchors.margins: units.gu(1)
> +                            text: model.name
> +                            color: textField.text === model.name ? UbuntuColors.orange : "black"
> +                        }
> +
> +                        onClicked: {
> +                            textField.text = model.name
> +                            textField.accept()

Fair enough. I tried with a "tag added" label but couldn't come up with something that looks/works reasonably well so I dropped that line.

> +                        }
> +                    }
> +                }
>              }
>          }
>  
> +
>          Button {
> -            text: i18n.tr("Create tag")
> +            id: okButton
> +            text: i18n.tr("OK")
>              color: UbuntuColors.orange
>              enabled: textField.text.replace(/\s+/g, '') !== ''; // Not only whitespaces!
>              onClicked: textField.accept()
> @@ -86,7 +141,7 @@
>          currentlyExpanded: true
>          multiSelection: true
>  
> -        containerHeight: Math.min(root.pageHeight - textField.height - closeButton.height - units.gu(12), tags.count * itemHeight)
> +        containerHeight: Math.min(root.pageHeight / 3, tags.count * itemHeight)
>  
>          model: tags
>  
> @@ -117,7 +172,7 @@
>  
>          color: UbuntuColors.orange
>  
> -        text: i18n.tr("Done")
> +        text: i18n.tr("Close")
>  
>          onClicked: {
>              root.done();
> 
> === modified file 'src/app/qml/reminders.qml'
> --- src/app/qml/reminders.qml	2015-06-01 09:56:30 +0000
> +++ src/app/qml/reminders.qml	2015-06-11 19:39:25 +0000
> @@ -383,7 +383,7 @@
>          onAuthenticated: {
>              EvernoteConnection.token = reply.AccessToken;
>              print("token is:", EvernoteConnection.token)
> -            if (NetworkingStatus.online) {
> +            if (NetworkingStatus.Online) {
>                  EvernoteConnection.connectToEvernote();
>              }
>          }
> 
> === modified file 'src/app/qml/ui/EditNoteView.qml'
> --- src/app/qml/ui/EditNoteView.qml	2015-05-30 16:18:42 +0000
> +++ src/app/qml/ui/EditNoteView.qml	2015-06-11 19:39:25 +0000
> @@ -647,7 +647,7 @@
>              RtfButton {
>                  iconName: "tick"
>                  // TRANSLATORS: Button to close the edit mode
> -                text: i18n.tr("Done")
> +                text: i18n.tr("Close")
>                  height: parent.height
>                  iconColor: UbuntuColors.green
>                  onClicked: {
> 
> === modified file 'src/libqtevernote/notesstore.cpp'
> --- src/libqtevernote/notesstore.cpp	2015-03-30 19:01:49 +0000
> +++ src/libqtevernote/notesstore.cpp	2015-06-11 19:39:25 +0000
> @@ -478,6 +478,12 @@
>  
>  Tag* NotesStore::createTag(const QString &name)
>  {
> +    foreach (Tag *tag, m_tags) {
> +        if (tag->name() == name) {
> +            return tag;
> +        }
> +    }
> +
>      QString newGuid = QUuid::createUuid().toString();
>      newGuid.remove("{").remove("}");
>      Tag *tag = new Tag(newGuid, 1, this);
> 


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