ubuntu-touch-coreapps-reviewers team mailing list archive

[Riccardo Padovani <riccardo@xxxxxxxxxxxxx>]

Review: Needs Fixing code

Last 2403 lines of diff (rev 90). 
Unfortunately they aren't here on Launchpad. So I locally merged this branch in the trunk, then I used qdiff, I report here interesting pieces:

src/app/qml/pdfView/PdfViewGotoDialog.qml
+    Button {
+        objectName:"GOButton"
+        text: i18n.tr("GO!")
+        color: UbuntuColors.orange

Orange isn't anymore suggested by design guidelines[0]. I suggest to use green or gray

###
src/app/qml/textView/TextView.qml

Why do you use a TextArea with readOnly: true and not a Label?

###
src/app/quick/documentmodel.cpp

Could you please explain what are you trying to achieve with DocumentModel::_q_directoryChanged?
Seems improvable, but I'm not sure I understand how you want to use it

DocumentModel::parseDirectoryContent
item.dateDiff = 0;
Probably is better to add a ENUM for this


Great work Stefano, congrats!

All issues I pointed out are minor, and the improvement is huge :-)

I'm going to test it

[0]https://design.ubuntu.com/apps/building-blocks/buttons
-- 
https://code.launchpad.net/~verzegnassi-stefano/ubuntu-docviewer-app/document-hub2/+merge/251166
Your team Ubuntu Document Viewer Developers is subscribed to branch lp:ubuntu-docviewer-app.