Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Photos on the map #4 #21

Merged
merged 12 commits into from
Sep 24, 2017
Merged

Photos on the map #4 #21

merged 12 commits into from
Sep 24, 2017

Conversation

BatPio
Copy link
Contributor

@BatPio BatPio commented Sep 6, 2017

Backend:

  • Extracting gps exif data from jpeg/tiff
  • Storing gps exif data in DB

UI:

  • Showing photos on map
  • Photos clustering (Leaflet/Leaflet.markercluster)
  • Photos previews on marker click

@tacruc
Copy link
Collaborator

tacruc commented Sep 13, 2017

Hi Toghether I was not able to get your branch running.
Trying to enable the app I get the following error
ParseError: syntax error, unexpected 'const' (T_CONST), expecting variable (T_VARIABLE) /var/www/server/lib/composer/composer/ClassLoader.php - line 322: Composer\Autoload\includeFile('/var/www/server...') [internal function] Composer\Autoload\ClassLoader->loadClass('OCA\\Maps\\Servic...') [internal function] spl_autoload_call('OCA\\Maps\\Servic...') /var/www/server/lib/private/AppFramework/Utility/SimpleContainer.php - line 94: ReflectionClass->__construct('OCA\\Maps\\Servic...') /var/www/server/lib/private/AppFramework/Utility/SimpleContainer.php - line 117: OC\AppFramework\Utility\SimpleContainer->resolve('OCA\\Maps\\Servic...') /var/www/server/lib/private/AppFramework/DependencyInjection/DIContainer.php - line 436: OC\AppFramework\Utility\SimpleContainer->query('OCA\\Maps\\Servic...') /var/www/server/lib/private/ServerContainer.php - line 116: OC\AppFramework\DependencyInjection\DIContainer->queryNoFallback('OCA\\Maps\\Servic...') /var/www/server/apps/maps/appinfo/application.php - line 31: OC\ServerContainer->query('OCA\\Maps\\Servic...') /var/www/server/3rdparty/pimple/pimple/src/Pimple/Container.php - line 113: OCA\Maps\AppInfo\Application->OCA\Maps\AppInfo\{closure}(Object(OC\AppFramework\DependencyInjection\DIContainer)) /var/www/server/lib/private/AppFramework/Utility/SimpleContainer.php - line 115: Pimple\Container->offsetGet('FileHooks') /var/www/server/lib/private/AppFramework/DependencyInjection/DIContainer.php - line 429: OC\AppFramework\Utility\SimpleContainer->query('FileHooks') /var/www/server/lib/private/AppFramework/DependencyInjection/DIContainer.php - line 414: OC\AppFramework\DependencyInjection\DIContainer->queryNoFallback('FileHooks') /var/www/server/apps/maps/appinfo/application.php - line 37: OC\AppFramework\DependencyInjection\DIContainer->query('FileHooks') /var/www/server/lib/private/Route/Router.php - line 375: OCA\Maps\AppInfo\Application->__construct() /var/www/server/lib/private/Route/Router.php - line 353: OC\Route\Router->setupRoutes(Array, 'maps') /var/www/server/lib/private/Route/Router.php - line 151: OC\Route\Router->requireRouteFile('/var/www/server...', 'maps') /var/www/server/lib/private/Route/Router.php - line 333: OC\Route\Router->loadRoutes() /var/www/server/lib/private/Route/CachingRouter.php - line 60: OC\Route\Router->generate('maps.page.index', Array, false) /var/www/server/lib/private/URLGenerator.php - line 77: OC\Route\CachingRouter->generate('maps.page.index', Array) /var/www/server/apps/maps/appinfo/app.php - line 25: OC\URLGenerator->linkToRoute('maps.page.index') /var/www/server/lib/private/legacy/app.php - line 209: require_once('/var/www/server...') /var/www/server/lib/private/legacy/app.php - line 149: OC_App requireAppFile('maps') /var/www/server/lib/private/legacy/app.php - line 124: OC_App loadApp('maps') /var/www/server/lib/base.php - line 989: OC_App loadApps() /var/www/server/index.php - line 48: OC handleRequest() {main}

@BatPio
Copy link
Contributor Author

BatPio commented Sep 13, 2017

@tacruc
Thank you for report. I wrote code using nextcloud on arch linux and it worked, but now I tested it on raspberry pi and I have same exception. Strange. Anyway, I pushed commit with fix. Can you try again?

@tacruc
Copy link
Collaborator

tacruc commented Sep 13, 2017

@BatPio
Enabling the app works fine now, thanks. But I get the maps Icon twice in the menu. Don't know if it has something to do with your changes or if it is a Problem somewhere else.
grafik

Should you allready see Pictures on the map? Because I can see non.

@BatPio
Copy link
Contributor Author

BatPio commented Sep 13, 2017

@tacruc
Doubled icon isn't result of my changes.
Photos must be scanned first using occ command "maps:scan-photos". Please remember that maps app is under heavy development and it's unsuitable for production usage yet.

@tacruc
Copy link
Collaborator

tacruc commented Sep 14, 2017

@BatPio Thanks for the hint, that's what I was missing. App is now running without any problems and errors.
I know that it is not stable I was just interested and installed it on my testing instance.

One suggestions. The Pictures open, when click on looks small to me, maybe you could make this window bigger, or add the possibility to open a preview when clicking on the enlarged image?

@BatPio
Copy link
Contributor Author

BatPio commented Sep 20, 2017

@tacruc
Fullscreen photo preview sounds good. I will check how it works in practice.

@v1r0x
Developing nextcloud/maps slowed down again. I hope not for long, I cant wait for functional maps app :)

@v1r0x
Copy link
Collaborator

v1r0x commented Sep 20, 2017

@v1r0x
Developing nextcloud/maps slowed down again. I hope not for long, I cant wait for functional maps app :)

Sorry for that. Wanted to test this branch a couple of days ago, but had no pictures at hand. Will look around for a couple of geo-referenced pictures and merge the PR.
Try to find some more time to work on maps, but on some days it is hard to get motivated to code after 8 hours of coding 😉

@v1r0x
Copy link
Collaborator

v1r0x commented Sep 20, 2017

Found a picture with gps data! :)
Uploaded it and run the occ command. The marker is there, but I see no image, just the "icon" of a not existing image. In the Log I get this one as error:
https://localhost/index.php/core/preview.png?file=/Documents/test.jpg&x=375&y=211&a=1
Any idea what's wrong?

@BatPio
Copy link
Contributor Author

BatPio commented Sep 21, 2017

I don't know why it don't work. Do you have enabled jpeg previews? Preview in files app is visible? Please attach screen with error. You can also try with my photos https://drive.google.com/file/d/0ByQ1JbfMQmtAdEVaa185TE5ESHM/view?usp=sharing

PS.
You don't have to rescan photos every time, file hook should automatically pin them to map after upload.

@v1r0x
Copy link
Collaborator

v1r0x commented Sep 21, 2017

Preview should be enabled by default, but I enabled it and added all image related preview providers. Thumbnail is visible in the files and gallery app.
I also ran the occ command several times and restarted the apache. I also tried to install ffmpeg, libjpeg and gd but they were already installed.

There is no error, just the 404 I get from /core/preview.png.

@BatPio
Copy link
Contributor Author

BatPio commented Sep 21, 2017

Please can you write what is the URL to thumbnail in files app? If it work in files app, it must be different than in my java script.

What version of Nextcloud are you using? I have 12.0.2

@BatPio BatPio requested review from v1r0x and removed request for v1r0x September 23, 2017 07:58

/* Preview size 32x32 is used in files view, so it sould be generated */
generateThumbnailUrl: function (filename) {
return "/index.php/core/preview.png?file=" + encodeURI(filename) + "&x=32&y=32";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

returning /index.php/... doesn't work if nextcloud isn't installed in the root dir. You can use OC.generateUrl('') to generate the base url or pass a path as parameter (OC.generateUrl('apps/maps/...')).
This fixed it for me:

return OC.generateUrl('core') + "/preview.png?file=" + encodeURI(filename) + "&x=32&y=32";


/* Preview size 375x211 is used in files details view */
generatePreviewUrl: function (filename) {
return "/index.php/core/preview.png?file=" + encodeURI(filename) + "&x=375&y=211&a=1";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

return OC.generateUrl('core') + "/preview.png?file=" + encodeURI(filename) + "&x=375&y=211&a=1";

},

getImageIconUrl: function() {
return "/index.php/apps/theming/img/core/filetypes/image.svg?v=2";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return OC.generateUrl('/apps/theming/img/core/filetypes') + "/image.svg?v=2";

@v1r0x
Copy link
Collaborator

v1r0x commented Sep 23, 2017

Please can you write what is the URL to thumbnail in files app? If it work in files app, it must be different than in my java script.

Good point. Didn't think about the obvious solution...Looking at the files app I saw the problem. You append /index.php/... while my nextcloud instance is at /nextcloud/index.php/.... I added comments to the lines in your JS code with a solution. Changing these lines gives the desired result. Nice! :)

@BatPio
Copy link
Contributor Author

BatPio commented Sep 23, 2017

Thanks for fixes. I added them to my code :D

@v1r0x
Copy link
Collaborator

v1r0x commented Sep 23, 2017

I think we can ignore the failed travis check, since it is related to gulp in the makefile which is currently not supported anyway.

Edit: If you agree you can merge it :)

@BatPio BatPio merged commit 0c5d478 into nextcloud:rework Sep 24, 2017
@BatPio
Copy link
Contributor Author

BatPio commented Sep 24, 2017

That's great! Merged :)

@v1r0x v1r0x mentioned this pull request Sep 24, 2017
@jancborchardt
Copy link
Member

Nice! Great collab @BatPio & @v1r0x! :) Hope we can do the same with the gpx apps by @eneiluj :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants