[Marble-devel] Review Request 124541: [MarbleMaps] Added search completion

[Dennis Nienhüser]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124541/#review83286
-----------------------------------------------------------

Ship it!


Awesome :-)
Only thing I'd change is adding an indentation to the search results such that they are left-aligned with the input field and the suggestions. The quickest way I can think of is doing it like this:

```
diff --git a/src/apps/marble-maps/Search.qml b/src/apps/marble-maps/Search.qml
index 99d47fc..50eb700 100644
--- a/src/apps/marble-maps/Search.qml
+++ b/src/apps/marble-maps/Search.qml
@@ -32,6 +32,7 @@ Item {
             left: parent.left
             right: parent.right
         }
+        leftIndent: Screen.pixelDensity * 3
         visible: false
         onItemSelected: {
             searchField.query = name;
diff --git a/src/apps/marble-maps/SearchResults.qml b/src/apps/marble-maps/SearchResults.qml
index a00c004..febfdbd 100644
--- a/src/apps/marble-maps/SearchResults.qml
+++ b/src/apps/marble-maps/SearchResults.qml
@@ -14,6 +14,8 @@ Item {
 
     signal itemSelected(int index, string name)
 
+    property int leftIndent: 0
+
     SystemPalette{
         id: palette
         colorGroup: SystemPalette.Active
@@ -28,11 +30,12 @@ Item {
     ListView {
         id: view
         anchors.fill: parent
+        anchors.leftMargin: root.leftIndent
         snapMode: ListView.SnapToItem
         model: root.model
         delegate: Item {
             width: view.width
-            height: placemarkName.height + 30
+            height: placemarkName.height + 20
 
             Rectangle {
                 id: delegateBackground
```
There might be a more elegant way.

Another nice thing to have would be showing icons next to search results. They are already in the model, but it needs some tweaking as QML strongly prefers taking care of icon loading itself (i.e. it wants paths, not QImage or QPixmap instances). A quick proof of concept patch is here https://paste.kde.org/prfmmeom6 but needs more work to sanitize placemark icon path handling inside Marble (wrt location and sizes). Let's deal with that outside of this review request.

http://nienhueser.de/marble/android-search-resultlist.png
![Search results with icons](http://nienhueser.de/marble/android-search-resultlist.png)

- Dennis Nienhüser


On July 31, 2015, 9:16 p.m., Mihail Ivchenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124541/
> -----------------------------------------------------------
> 
> (Updated July 31, 2015, 9:16 p.m.)
> 
> 
> Review request for Marble and Gábor Péterffy.
> 
> 
> Repository: marble
> 
> 
> Description
> -------
> 
> Added search completion for Marble Maps.
> 
> 
> Diffs
> -----
> 
>   src/lib/marble/declarative/SearchBackend.cpp 5c80cc3 
>   data/android/drawable-xxxhdpi/search.png 599a3c7ccdcedb11835378562f7f34c2a4c39669 
>   src/apps/marble-maps/Completion.qml PRE-CREATION 
>   src/apps/marble-maps/MainScreen.qml 2cc996a 
>   src/apps/marble-maps/MarbleMaps.qrc d027af9 
>   src/apps/marble-maps/Search.qml 896f266 
>   src/apps/marble-maps/SearchField.qml 5830098 
>   src/apps/marble-maps/SearchResults.qml a910038 
>   src/apps/marble-maps/package/AndroidManifest.xml 4409ec5 
>   src/lib/marble/declarative/SearchBackend.h b041de3 
> 
> Diff: https://git.reviewboard.kde.org/r/124541/diff/
> 
> 
> Testing
> -------
> 
> Works on Nexus 5 and Nexus 9 (both are Android 5.1.1)
> 
> I'm not sure about color for background so I created a screenshot which shows all available colors from SystemPalette with default Holo Dark theme on stock Android 5.1.1.
> 
> 
> File Attachments
> ----------------
> 
> Screenshot_2015-08-01-01-06-06.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/07/31/a115b5d5-7f57-4089-a896-d8fbf2ed2320__Screenshot_2015-08-01-01-06-06.png
> Screenshot_2015-08-01-01-06-12.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/07/31/7df5b460-e705-4da3-a846-a9d308f37daf__Screenshot_2015-08-01-01-06-12.png
> Screenshot_2015-08-01-01-06-16.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/07/31/386444f1-8d4e-43de-96fd-9810f91e5a14__Screenshot_2015-08-01-01-06-16.png
> Screenshot_2015-08-01-01-06-22.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/07/31/490fe2b2-8cf3-4456-82c1-08b9a81dcc89__Screenshot_2015-08-01-01-06-22.png
> Screenshot_2015-08-01-01-06-26.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/07/31/6d497bac-01ea-424d-a636-da1b2a5d0311__Screenshot_2015-08-01-01-06-26.png
> 
> 
> Thanks,
> 
> Mihail Ivchenko
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20150801/f47bd355/attachment-0001.html>