[Marble-devel] Review Request 124466: [MarbleMaps] Add search bar and "Theme item" for Marble Maps.

Sun Jul 26 08:59:01 UTC 2015

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

Works fine on a Nexus 4 also.

Regarding the startup time: This happens as much more plugins are compiled on Android now. I plan to rework the package creation a bit next to allow certain files (plugins, data components) which are not needed to be excluded from the package.

src/apps/marble-maps/CMakeLists.txt (line 1)
<https://git.reviewboard.kde.org/r/124466/#comment57264>

    Can you add the new .qml files to the custom target so that IDEs like QtCreator show them in the project tree?

    ```
    diff --git a/src/apps/marble-maps/CMakeLists.txt b/src/apps/marble-maps/CMakeLists.txt
    index 380bbf3..c7c1437 100644
    --- a/src/apps/marble-maps/CMakeLists.txt
    +++ b/src/apps/marble-maps/CMakeLists.txt
    @@ -29,7 +29,8 @@ file(COPY "../../../data/android/drawable-xhdpi" DESTINATION "${CMAKE_BINARY_DIR
     file(COPY "../../../data/android/drawable-xxhdpi" DESTINATION "${CMAKE_BINARY_DIR}/src/apps/marble-maps/res")
     file(COPY "../../../data/android/drawable-xxxhdpi" DESTINATION "${CMAKE_BINARY_DIR}/src/apps/marble-maps/res")

    -add_custom_target(marble-maps_resources ALL SOURCES MainScreen.qml package/AndroidManifest.xml)
    +FILE (GLOB QML_FILES *.qml)
    +add_custom_target(marble-maps_resources ALL SOURCES ${QML_FILES} package/AndroidManifest.xml)

     if (ANDROID)
         include("$ENV{Qt5_android_cmake}/AddQtAndroidApk.cmake")
    ```

src/apps/marble-maps/MainScreen.qml (line 18)
<https://git.reviewboard.kde.org/r/124466/#comment57259>

    I wonder if we should use properties instead of functions, see below.

src/apps/marble-maps/MarbleMaps.qrc (line 8)
<https://git.reviewboard.kde.org/r/124466/#comment57265>

    We should add the usage of Android material icons to our about / credit dialog. It doesn't exist yet for Android, so let's ignore it for now, but work on it soon.

src/apps/marble-maps/Search.qml (line 14)
<https://git.reviewboard.kde.org/r/124466/#comment57263>

    Visibility changes propagate to children, so we could simplify it like this: https://paste.kde.org/pnhzu51b8

src/apps/marble-maps/SearchBackend.cpp (line 73)
<https://git.reviewboard.kde.org/r/124466/#comment57266>

    The check is not needed, calling delete on a nullptr is perfectly fine.

- Dennis Nienhüser

On July 25, 2015, 9:08 p.m., Mihail Ivchenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124466/
> -----------------------------------------------------------
> 
> (Updated July 25, 2015, 9:08 p.m.)
> 
> 
> Review request for Marble and Gábor Péterffy.
> 
> 
> Repository: marble
> 
> 
> Description
> -------
> 
> Based on [work of Gábor Péterffy](https://git.reviewboard.kde.org/r/124327/)
> 
> 
> Diffs
> -----
> 
>   data/android/drawable-xxxhdpi/search.png PRE-CREATION 
>   src/apps/marble-maps/CMakeLists.txt ea6ca1e 
>   src/apps/marble-maps/MainScreen.qml 8f50962 
>   src/apps/marble-maps/MarbleMaps.qrc 6bb2336 
>   src/apps/marble-maps/Search.qml PRE-CREATION 
>   src/apps/marble-maps/SearchBackend.h PRE-CREATION 
>   src/apps/marble-maps/SearchBackend.cpp PRE-CREATION 
>   src/apps/marble-maps/SearchField.qml PRE-CREATION 
>   src/apps/marble-maps/SearchResults.qml PRE-CREATION 
>   src/apps/marble-maps/Theme.qml PRE-CREATION 
>   src/apps/marble-maps/main.cpp 08f4be5 
>   src/lib/marble/MarblePlacemarkModel.cpp b814fdb 
>   src/lib/marble/MarbleQuickItem.h fa61707 
> 
> Diff: https://git.reviewboard.kde.org/r/124466/diff/
> 
> 
> Testing
> -------
> 
> Works well on Nexus 5 & Nexus 9 (both are Android 5.1.1).
> Only issue I noticed:
> After starting UI doesn't for few sec (looks like because of plugins loading). But that issue wasn't caused by this patch but previous one.
> 
> 
> File Attachments
> ----------------
> 
> search.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/07/25/e875539c-62ec-4b71-a49c-20f477187eed__search.png
> Screenshot_2015-07-26-00-42-05.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/07/25/835ab63a-aca5-4efa-a868-601b8b15f70e__Screenshot_2015-07-26-00-42-05.png
> Screenshot_2015-07-26-00-42-15.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/07/25/977187a9-0fc3-43bf-b050-d819476c2afb__Screenshot_2015-07-26-00-42-15.png
> Screenshot_2015-07-26-00-42-28.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/07/25/357df10d-5170-4e58-808f-6fece05327f6__Screenshot_2015-07-26-00-42-28.png
> Screenshot_2015-07-26-00-42-50.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/07/25/ce250cea-47e9-4829-b705-370daf431742__Screenshot_2015-07-26-00-42-50.png
> Screenshot_2015-07-26-00-43-05.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/07/25/e5ebe15e-b394-42ce-a2d1-ba729cf00780__Screenshot_2015-07-26-00-43-05.png
> 
> 
> Thanks,
> 
> Mihail Ivchenko
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20150726/8c741cd9/attachment.html>