ubuntu-touch-coreapps-reviewers team mailing list archive
-
ubuntu-touch-coreapps-reviewers team
-
Mailing list archive
-
Message #02997
Re: [Merge] lp:~mzanetti/reminders-app/contenthub-import into lp:reminders-app
Review: Needs Fixing
Looks good (just three little things to change if you don't mind).
Diff comments:
> === modified file 'CMakeLists.txt'
> --- CMakeLists.txt 2015-02-16 22:01:20 +0000
> +++ CMakeLists.txt 2015-06-15 00:00:45 +0000
> @@ -74,6 +74,7 @@
> reminders.url-dispatcher
> push-helper.json
> push-helper.apparmor
> + reminders-contenthub.json
> DESTINATION ${CMAKE_INSTALL_PREFIX})
> install(FILES COPYING DESTINATION ${CMAKE_INSTALL_PREFIX})
> else(CLICK_MODE)
>
> === modified file 'manifest.json.in'
> --- manifest.json.in 2015-03-20 11:16:35 +0000
> +++ manifest.json.in 2015-06-15 00:00:45 +0000
> @@ -8,7 +8,8 @@
> "account-service": "@ACCOUNT_SERVICE_DIR@/com.ubuntu.reminders_reminders.service",
> "apparmor": "reminders.apparmor",
> "desktop": "com.ubuntu.reminders.desktop",
> - "urls": "reminders.url-dispatcher"
> + "urls": "reminders.url-dispatcher",
> + "content-hub": "reminders-contenthub.json"
> },
> "evernote-account-plugin": {
> "account-qml-plugin": "@ACCOUNT_QML_PLUGIN_DIR@/evernote",
>
> === added file 'reminders-contenthub.json'
> --- reminders-contenthub.json 1970-01-01 00:00:00 +0000
> +++ reminders-contenthub.json 2015-06-15 00:00:45 +0000
> @@ -0,0 +1,8 @@
> +{
> + "destination": [
> + "pictures",
> + "links",
> + "text"
> + ]
> +}
> +
>
> === modified file 'src/app/qml/components/NotesDelegate.qml'
> --- src/app/qml/components/NotesDelegate.qml 2015-03-15 20:00:21 +0000
> +++ src/app/qml/components/NotesDelegate.qml 2015-06-15 00:00:45 +0000
> @@ -136,7 +136,7 @@
> Layout.fillHeight: true
> text: "<font color=\"" + root.notebookColor + "\">" +
> Qt.formatDateTime(root.date, Qt.LocalDate) +
> - " </font>" + root.content
> + " </font>" + root.content.replace("\n", " ").trim()
> wrapMode: Text.WordWrap
> textFormat: Text.StyledText
> maximumLineCount: 2
>
> === modified file 'src/app/qml/components/StatusBar.qml'
> --- src/app/qml/components/StatusBar.qml 2015-03-08 21:29:28 +0000
> +++ src/app/qml/components/StatusBar.qml 2015-06-15 00:00:45 +0000
> @@ -10,36 +10,50 @@
>
> property bool shown: false
> property alias iconName: icon.name
> + property alias iconColor: icon.color
> property alias text: label.text
> + property alias showCancelButton: cancelButton.visible
> +
> + signal cancel();
>
> Behavior on height {
> UbuntuNumberAnimation {}
> }
>
> - ColumnLayout {
> + RowLayout {
> id: statusBarContents
> - anchors { left: parent.left; top: parent.top; right: parent.right }
> + anchors { left: parent.left; right: parent.right; leftMargin: units.gu(1); rightMargin: units.gu(1); verticalCenter: parent.verticalCenter }
> spacing: units.gu(1)
> -
> - Row {
> - anchors { left: parent.left; right: parent.right; margins: units.gu(1) }
> + Column {
> spacing: units.gu(1)
> - height: label.height
> -
> - Icon {
> - id: icon
> - height: units.gu(3)
> - width: height
> - color: UbuntuColors.red
> - anchors.verticalCenter: parent.verticalCenter
> - }
> -
> - Label {
> - id: label
> - width: parent.width - x
> - wrapMode: Text.WordWrap
> - anchors.verticalCenter: parent.verticalCenter
> - }
> + Layout.fillWidth: true
> +
> + Row {
> + anchors { left: parent.left; right: parent.right }
> + spacing: units.gu(1)
> + height: label.height
> +
> + Icon {
> + id: icon
> + height: units.gu(3)
> + width: height
> + anchors.verticalCenter: parent.verticalCenter
> + }
> +
> + Label {
> + id: label
> + width: parent.width - x
> + wrapMode: Text.WordWrap
> + anchors.verticalCenter: parent.verticalCenter
> + }
> + }
> + }
> +
> + Button {
> + id: cancelButton
> + Layout.preferredWidth: height
> + iconName: "cancel"
> + onClicked: root.cancel();
> }
> }
> }
>
> === modified file 'src/app/qml/reminders.qml'
> --- src/app/qml/reminders.qml 2015-06-12 09:48:22 +0000
> +++ src/app/qml/reminders.qml 2015-06-15 00:00:45 +0000
> @@ -26,6 +26,7 @@
> import Ubuntu.OnlineAccounts 0.1
> import Ubuntu.OnlineAccounts.Client 0.1
> import Ubuntu.PushNotifications 0.1
> +import Ubuntu.Content 1.0
> import "components"
> import "ui"
>
> @@ -79,6 +80,46 @@
> }
> }
>
> + property var importTransfer: null
> + function handleImportTransfer(note) {
> + if (importTransfer == null) return;
> +
> + for (var i = 0; i < importTransfer.items.length; i++) {
> + var url = importTransfer.items[i].url;
> + switch (importTransfer.contentType) {
> + case ContentType.Links:
> + note.insertLink(note.plaintextContent.length, url)
> + break;
> + default:
> + note.attachFile(note.plaintextContent.length, url)
> + break;
> + }
> + }
> + note.save();
> + importTransfer = null;
> + }
> +
> + Connections {
> + target: ContentHub
> + onImportRequested: {
> + importTransfer = transfer
> + var popup = PopupUtils.open(importQuestionComponent);
> + popup.accepted.connect(function(createNew) {
> + PopupUtils.close(popup)
> + if (createNew) {
> + var note = NotesStore.createNote(i18n.tr("Untitled"))
Nitpick: you forgot the ';'
> + handleImportTransfer(note);
> + }
> + })
> +
> + popup.rejected.connect(function() {
> + PopupUtils.close(popup)
> + importTransfer = null;
> + })
> +
> + }
> + }
> +
> Timer {
> id: connectDelayTimer
> interval: 2000
> @@ -107,6 +148,10 @@
> conflictMode = false;
> }
>
> + if (importTransfer != null) {
> + handleImportTransfer(note);
> + }
> +
> print("displayNote:", note.guid)
> note.load(true);
> if (root.narrowMode) {
> @@ -436,28 +481,46 @@
> }
> }
>
> - StatusBar {
> + Column {
> id: statusBar
> anchors { left: parent.left; right: parent.right; top: parent.top; topMargin: units.gu(9) }
> - color: root.backgroundColor
> - shown: text
> - text: EvernoteConnection.error || NotesStore.error
> - iconName: "sync-error"
> -
> - Timer {
> - interval: 5000
> - repeat: true
> - running: NotesStore.error
> - onTriggered: NotesStore.clearError();
> - }
> -
> - MouseArea {
> - anchors.fill: parent
> - onClicked: NotesStore.clearError();
> - }
> -
> +
> + StatusBar {
> + anchors { left: parent.left; right: parent.right }
> + color: root.backgroundColor
> + shown: text
> + text: EvernoteConnection.error || NotesStore.error
> + iconName: "sync-error"
> + iconColor: UbuntuColors.red
> + showCancelButton: true
> +
> + onCancel: {
> + NotesStore.clearError();
> + }
> +
> + Timer {
> + interval: 5000
> + repeat: true
> + running: NotesStore.error
> + onTriggered: NotesStore.clearError();
> + }
> + }
> +
> + StatusBar {
> + anchors { left: parent.left; right: parent.right }
> + color: root.backgroundColor
> + shown: importTransfer != null
> + text: importTransfer.items.length === 1 ? i18n.tr("Select note to attach imported file") : i18n.tr("Select note to attach imported files")
> + iconName: "document-save"
> + showCancelButton: true
> +
> + onCancel: {
> + importTransfer = null;
> + }
> + }
> }
>
> +
Nitpick, this is useless :-)
> PageStack {
> id: pagestack
> anchors.rightMargin: root.narrowMode ? 0 : root.width - units.gu(40)
> @@ -673,4 +736,35 @@
> }
> }
> }
> +
> + Component {
> + id: importQuestionComponent
> +
> + Dialog {
> + id: importDialog
> + title: importTransfer.items.length > 1 ?
> + i18n.tr("Importing %1 items").arg(importTransfer.items.length)
> + : i18n.tr("Importing 1 item")
> + text: i18n.tr("Do you want to create a new note for this item or do you want to attach it to an existing note?")
I think you should create the single/plural option also there (this item/these items)
> +
> + signal accepted(bool createNew);
> + signal rejected();
> +
> + Button {
> + text: i18n.tr("Create new note")
> + onClicked: importDialog.accepted(true)
> + color: UbuntuColors.green
> + }
> + Button {
> + text: i18n.tr("Attach to existing note")
> + onClicked: importDialog.accepted(false);
> + color: UbuntuColors.blue
> + }
> + Button {
> + text: i18n.tr("Cancel import")
> + onClicked: importDialog.rejected();
> + color: UbuntuColors.red
> + }
> + }
> + }
> }
>
> === modified file 'src/app/qml/ui/NotesPage.qml'
> --- src/app/qml/ui/NotesPage.qml 2015-06-11 22:02:25 +0000
> +++ src/app/qml/ui/NotesPage.qml 2015-06-15 00:00:45 +0000
> @@ -142,15 +142,12 @@
> }
>
> function sortOrderToString(sortOrder){
> - print("asking for sortOrder", sortOrder, Notes.SortOrderDateUpdatedNewest)
> switch(sortOrder) {
> case Notes.SortOrderDateCreatedNewest:
> case Notes.SortOrderDateCreatedOldest:
> - print("returning createdString")
> return "createdString";
> case Notes.SortOrderDateUpdatedNewest:
> case Notes.SortOrderDateUpdatedOldest:
> - print("returning updatedString")
> return "updatedString";
> case Notes.SortOrderTitleAscending:
> case Notes.SortOrderTitleDescending:
>
> === modified file 'src/libqtevernote/note.cpp'
> --- src/libqtevernote/note.cpp 2015-06-03 17:45:45 +0000
> +++ src/libqtevernote/note.cpp 2015-06-15 00:00:45 +0000
> @@ -277,7 +277,7 @@
>
> QString Note::plaintextContent() const
> {
> - return m_content.toPlaintext().trimmed();
> + return m_content.toPlaintext();
> }
>
> QString Note::tagline() const
> @@ -570,6 +570,20 @@
> NotesStore::instance()->untagNote(m_guid, tagGuid);
> }
>
> +void Note::insertText(int position, const QString &text)
> +{
> + m_content.insertText(position, text);
> + m_tagline = m_content.toPlaintext().left(100);
> + emit contentChanged();
> +}
> +
> +void Note::insertLink(int position, const QString &url)
> +{
> + m_content.insertLink(position, url);
> + m_tagline = m_content.toPlaintext().left(100);
> + emit contentChanged();
> +}
> +
> int Note::renderWidth() const
> {
> return m_content.renderWidth();
>
> === modified file 'src/libqtevernote/note.h'
> --- src/libqtevernote/note.h 2015-03-15 20:00:21 +0000
> +++ src/libqtevernote/note.h 2015-06-15 00:00:45 +0000
> @@ -165,6 +165,8 @@
> Q_INVOKABLE void attachFile(int position, const QUrl &fileName);
> Q_INVOKABLE void addTag(const QString &tagGuid);
> Q_INVOKABLE void removeTag(const QString &tagGuid);
> + Q_INVOKABLE void insertText(int position, const QString &text);
> + Q_INVOKABLE void insertLink(int position, const QString &url);
>
> int renderWidth() const;
> void setRenderWidth(int renderWidth);
>
> === modified file 'src/libqtevernote/utils/enmldocument.cpp'
> --- src/libqtevernote/utils/enmldocument.cpp 2015-04-13 20:42:38 +0000
> +++ src/libqtevernote/utils/enmldocument.cpp 2015-06-15 00:00:45 +0000
> @@ -486,48 +486,161 @@
>
> void EnmlDocument::attachFile(int position, const QString &hash, const QString &type)
> {
> - QXmlStreamReader reader(m_enml);
> -
> - QString output;
> - QXmlStreamWriter writer(&output);
> - writer.writeStartDocument();
> - writer.writeDTD("<!DOCTYPE en-note SYSTEM \"http://xml.evernote.com/pub/enml2.dtd\";>");
> -
> - int textPos = 0;
> - bool inserted = false;
> -
> - while (!reader.atEnd() && !reader.hasError()) {
> - QXmlStreamReader::TokenType token = reader.readNext();
> -
> - if (token == QXmlStreamReader::StartElement) {
> - writer.writeStartElement(reader.name().toString());
> - writer.writeAttributes(reader.attributes());
> - }
> -
> - if (token == QXmlStreamReader::Characters) {
> - QString textString = reader.text().toString();
> - if (textPos <= position && textPos + textString.length() > position) {
> - writer.writeCharacters(textString.left(position - textPos));
> -
> - writer.writeStartElement("en-media");
> - writer.writeAttribute("hash", hash);
> - writer.writeAttribute("type", type);
> - writer.writeEndElement();
> - inserted = true;
> -
> - writer.writeCharacters(textString.right(textString.length() - (position - textPos)));
> - } else {
> - writer.writeCharacters(reader.text().toString());
> - }
> - textPos += textString.length();
> - }
> - if (token == QXmlStreamReader::EndElement) {
> -
> - // The above logic would fail on an empty note
> - if (reader.name() == "en-note" && !inserted) {
> - writer.writeStartElement("en-media");
> - writer.writeAttribute("hash", hash);
> - writer.writeAttribute("type", type);
> + qCDebug(dcEnml) << "Attaching file at position" << position;
> + QXmlStreamReader reader(m_enml);
> +
> + QString output;
> + QXmlStreamWriter writer(&output);
> + writer.writeStartDocument();
> + writer.writeDTD("<!DOCTYPE en-note SYSTEM \"http://xml.evernote.com/pub/enml2.dtd\";>");
> +
> + int textPos = 0;
> + bool inserted = false;
> +
> + while (!reader.atEnd() && !reader.hasError()) {
> + QXmlStreamReader::TokenType token = reader.readNext();
> +
> + if (token == QXmlStreamReader::StartElement) {
> + writer.writeStartElement(reader.name().toString());
> + writer.writeAttributes(reader.attributes());
> + }
> +
> + if (token == QXmlStreamReader::Characters) {
> + QString textString = reader.text().toString();
> + if (textPos <= position && textPos + textString.length() > position) {
> + writer.writeCharacters(textString.left(position - textPos));
> +
> + writer.writeStartElement("en-media");
> + writer.writeAttribute("hash", hash);
> + writer.writeAttribute("type", type);
> + writer.writeEndElement();
> + inserted = true;
> +
> + writer.writeCharacters(textString.right(textString.length() - (position - textPos)));
> + } else {
> + writer.writeCharacters(reader.text().toString());
> + }
> + textPos += textString.length();
> + }
> + if (token == QXmlStreamReader::EndElement) {
> +
> + // The above logic would fail on an empty note
> + if (reader.name() == "en-note" && !inserted) {
> + writer.writeStartElement("en-media");
> + writer.writeAttribute("hash", hash);
> + writer.writeAttribute("type", type);
> + writer.writeEndElement();
> + }
> +
> + writer.writeEndElement();
> + }
> + }
> + m_enml = output;
> +}
> +
> +void EnmlDocument::insertText(int position, const QString &text)
> +{
> + qCDebug(dcEnml) << "Inserting Text at position" << position;
> +
> + QXmlStreamReader reader(m_enml);
> +
> + QString output;
> + QXmlStreamWriter writer(&output);
> + writer.writeStartDocument();
> + writer.writeDTD("<!DOCTYPE en-note SYSTEM \"http://xml.evernote.com/pub/enml2.dtd\";>");
> +
> + int textPos = 0;
> + bool inserted = false;
> +
> + while (!reader.atEnd() && !reader.hasError()) {
> + QXmlStreamReader::TokenType token = reader.readNext();
> +
> + if (token == QXmlStreamReader::StartElement) {
> + writer.writeStartElement(reader.name().toString());
> + writer.writeAttributes(reader.attributes());
> + }
> +
> + if (token == QXmlStreamReader::Characters) {
> + QString textString = reader.text().toString();
> + if (textPos <= position && textPos + textString.length() > position) {
> + writer.writeCharacters(textString.left(position - textPos));
> +
> + writer.writeStartElement("div");
> + writer.writeCharacters(text);
> + writer.writeEndElement();
> + inserted = true;
> +
> + writer.writeCharacters(textString.right(textString.length() - (position - textPos)));
> + } else {
> + writer.writeCharacters(reader.text().toString());
> + }
> + textPos += textString.length();
> + }
> + if (token == QXmlStreamReader::EndElement) {
> +
> + // The above logic would fail on an empty note
> + if (reader.name() == "en-note" && !inserted) {
> + writer.writeStartElement("div");
> + writer.writeCharacters(text);
> + writer.writeEndElement();
> + }
> +
> + writer.writeEndElement();
> + }
> + }
> + m_enml = output;
> +}
> +
> +void EnmlDocument::insertLink(int position, const QString &url)
> +{
> + qCDebug(dcEnml) << "Inserting Link at position" << position;
> +
> + QXmlStreamReader reader(m_enml);
> +
> + QString output;
> + QXmlStreamWriter writer(&output);
> + writer.writeStartDocument();
> + writer.writeDTD("<!DOCTYPE en-note SYSTEM \"http://xml.evernote.com/pub/enml2.dtd\";>");
> +
> + int textPos = 0;
> + bool inserted = false;
> +
> + while (!reader.atEnd() && !reader.hasError()) {
> + QXmlStreamReader::TokenType token = reader.readNext();
> +
> + if (token == QXmlStreamReader::StartElement) {
> + writer.writeStartElement(reader.name().toString());
> + writer.writeAttributes(reader.attributes());
> + }
> +
> + if (token == QXmlStreamReader::Characters) {
> + QString textString = reader.text().toString();
> + if (textPos <= position && textPos + textString.length() > position) {
> + writer.writeCharacters(textString.left(position - textPos));
> +
> + writer.writeStartElement("div");
> + writer.writeStartElement("a");
> + writer.writeAttribute("href", url);
> + writer.writeCharacters(url);
> + writer.writeEndElement();
> + writer.writeEndElement();
> + inserted = true;
> +
> + writer.writeCharacters(textString.right(textString.length() - (position - textPos)));
> + } else {
> + writer.writeCharacters(reader.text().toString());
> + }
> + textPos += textString.length();
> + }
> + if (token == QXmlStreamReader::EndElement) {
> +
> + // The above logic would fail on an empty note
> + if (reader.name() == "en-note" && !inserted) {
> + writer.writeStartElement("div");
> + writer.writeStartElement("a");
> + writer.writeAttribute("href", url);
> + writer.writeCharacters(url);
> + writer.writeEndElement();
> writer.writeEndElement();
> }
>
> @@ -551,10 +664,8 @@
> // Write all normal text inside <body> </body> to output
> if (token == QXmlStreamReader::Characters) {
> plaintext.append(reader.text().toString());
> - plaintext.append(' ');
> }
> }
>
> - plaintext.remove('\n').trimmed();
> return plaintext;
> }
>
> === modified file 'src/libqtevernote/utils/enmldocument.h'
> --- src/libqtevernote/utils/enmldocument.h 2015-03-05 18:23:25 +0000
> +++ src/libqtevernote/utils/enmldocument.h 2015-06-15 00:00:45 +0000
> @@ -41,6 +41,10 @@
> // Will insert the file described by hash at position in the plaintext string
> void attachFile(int position, const QString &hash, const QString &type);
>
> + // Convenience functions to insert some text without having a complete Editor attached
> + void insertText(int position, const QString &text);
> + void insertLink(int position, const QString &url);
> +
> void markTodo(const QString &todoId, bool checked);
>
> int renderWidth() const;
>
--
https://code.launchpad.net/~mzanetti/reminders-app/contenthub-import/+merge/261919
Your team Ubuntu Reminders app developers is subscribed to branch lp:reminders-app.
References
