----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124327/#review82654 -----------------------------------------------------------
src/apps/marble_maps/SearchBar.qml (line 22) <https://git.reviewboard.kde.org/r/124327/#comment57015> I don't think we need z-values in here at all. src/apps/marble_maps/SearchBar.qml (line 26) <https://git.reviewboard.kde.org/r/124327/#comment57019> height should be searchButton.height However I'd really prefer anchors to width/height calculations. It's much easier to maintain. The effect you want is visually like this | text input |O| with O the button Try setting up anchors like this: inputField.left => parent.left inputField.right => searchButton.left searchButton.right => parent.right and finally a proper width for searchButton. This way all information is there needed to calculate the (horizontal) positions and the width of both elements, and there is no redundant information. You can also easily introduce margins (anchors.margins or anchors.marginLeft, ...) for fine-tuning. See http://doc.qt.io/qt-5/qtquick-positioning-anchors.html for details. http://doc.qt.io/qt-5/qtquick-positioning-topic.html is a good read also for the more general concepts. src/apps/marble_maps/SearchBar.qml (line 69) <https://git.reviewboard.kde.org/r/124327/#comment57014> I think using anchors+margins in inputField would be nicer and achieves the same result (see above) src/apps/marble_maps/SearchBar.qml (line 82) <https://git.reviewboard.kde.org/r/124327/#comment57013> I wonder if we should introduce some common ColorPalette.qml or similar, which has colors as named properties. Makes it much easier to read and also to maintain. src/apps/marble_maps/SearchBarBackend.h (line 48) <https://git.reviewboard.kde.org/r/124327/#comment57011> Seems unused src/apps/marble_maps/SearchBarBackend.cpp (line 20) <https://git.reviewboard.kde.org/r/124327/#comment57012> also initialize m_placemarkModel to nullptr here src/apps/marble_maps/SearchBarBackend.cpp (line 64) <https://git.reviewboard.kde.org/r/124327/#comment57010> I'd delete m_searchManager before (just in case the method is called more than once) src/plugins/runner/openrouteservice/OpenRouteServiceRunner.cpp <https://git.reviewboard.kde.org/r/124327/#comment57020> Can you push this and the similar change in YoursRunner.cpp directly to master (no review request needed)? - Dennis Nienhüser On July 19, 2015, 3:44 p.m., Gábor Péterffy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124327/ > ----------------------------------------------------------- > > (Updated July 19, 2015, 3:44 p.m.) > > > Review request for Marble. > > > Repository: marble > > > Description > ------- > > This patch adds a search bar to Marble Maps. Because the loading of the > plugins not works on Android, the search function is not working. > > > Diffs > ----- > > data/android/pixmaps/searchFocused.png PRE-CREATION > data/android/pixmaps/searchPressed.png PRE-CREATION > data/android/pixmaps/searchReleased.png PRE-CREATION > src/apps/marble_maps/CMakeLists.txt PRE-CREATION > src/apps/marble_maps/MainScreen.qml PRE-CREATION > src/apps/marble_maps/MarbleMaps.qrc PRE-CREATION > src/apps/marble_maps/QmlView.h PRE-CREATION > src/apps/marble_maps/QmlView.cpp PRE-CREATION > src/apps/marble_maps/SearchBar.qml PRE-CREATION > src/apps/marble_maps/SearchBarBackend.h PRE-CREATION > src/apps/marble_maps/SearchBarBackend.cpp PRE-CREATION > src/apps/marble_maps/main.cpp PRE-CREATION > src/lib/marble/MarblePlacemarkModel.cpp > a2c78c4f8603752182f64e42e02a585d5ea6b2e3 > src/lib/marble/MarbleQuickItem.h 41e20bb69bbeb9c1d662cd943ecb13674fd04498 > src/plugins/runner/openrouteservice/OpenRouteServiceRunner.cpp > 7c1803998ce5a01e338f2eb04a2b7f310a926c26 > src/plugins/runner/yours/YoursRunner.cpp > 4b1c0552718acc4a2f3070e95950975a57d4fdd4 > > Diff: https://git.reviewboard.kde.org/r/124327/diff/ > > > Testing > ------- > > On linux it works fine when the runner plugins are loaded. On Android the UI > appears fine but not working with loaded plugins. > > > File Attachments > ---------------- > > Screenshot > > https://git.reviewboard.kde.org/media/uploaded/files/2015/07/13/95493cb2-c43c-48f4-a1f0-7a5ca0486856__Screenshot_2015-07-13-17-57-06.png > Screenshot 2 > > https://git.reviewboard.kde.org/media/uploaded/files/2015/07/13/ab43529f-26f4-4f52-9ba5-1c759fcee07d__Screenshot_2015-07-13-17-56-45.png > Screenshot 3 > > https://git.reviewboard.kde.org/media/uploaded/files/2015/07/13/b6c76a6b-9118-4841-97e7-c204567454b8__Screenshot_2015-07-13-17-40-46.png > searchFocused.png > > https://git.reviewboard.kde.org/media/uploaded/files/2015/07/19/4fc5e41a-40aa-4d62-879c-6436e93dd1a0__searchFocused.png > searchPressed.png > > https://git.reviewboard.kde.org/media/uploaded/files/2015/07/19/c16c83c4-d25f-482d-b409-7cece2f7bad0__searchPressed.png > searchReleased.png > > https://git.reviewboard.kde.org/media/uploaded/files/2015/07/19/58b3d964-641d-457f-911e-62066d55cf61__searchReleased.png > > > Thanks, > > Gábor Péterffy > >
_______________________________________________ Marble-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/marble-devel
