Skip to content
This repository has been archived by the owner on Nov 1, 2020. It is now read-only.

Exif description #426

Closed
wants to merge 8 commits into from
Closed

Exif description #426

wants to merge 8 commits into from

Conversation

turb
Copy link

@turb turb commented Apr 8, 2018

Licence: MIT or AGPL

Description

Port of @0xb0ba Exif Description PR to Nextcloud gallery.

Features

  • replaces the display of the filename with any description found in the image EXIF properties, when displaying a picture

Screenshots or screencasts

See owncloud/gallery#453

Tests

Test plan

Tested with exif:ImageDescription set by Apple.

Not tested with iptc:APP13 (IPTC:Description) (?) set by other manufacturers like Adobe.

Tested on

  • Mac OS X/Firefox
  • Mac OS X/Safari
  • Mac OS X/Chromium
  • Android 4.4/Chrome

TODO

  • also display it on each caption in the gallery?

Check list

  • Code is properly documented
  • Code is properly formatted
  • Commits have been squashed
  • Tests (unit, integration, api and/or acceptance) are included
  • Documentation (manuals or wiki) has been updated or is not required

Reviewers

@oparoz
Copy link
Member

oparoz commented Apr 9, 2018

My main problem with this is that this is parsing the exif data for every image and will slow down the rendering for people who do not care about showing exif data.

Maybe we could make it optional using a configuration switch in gallery.cnf while we wait for exif data to be properly collected and stored in the DB at upload time.

@turb
Copy link
Author

turb commented Apr 9, 2018

@oparoz the EXIF extraction is triggered by a JS async call after the rendering, to it should not slow it.

I do not know enough about OC/NC code to implement DB storage right now, maybe someone can.

@oparoz
Copy link
Member

oparoz commented Apr 9, 2018

@oparoz the EXIF extraction is triggered by a JS async call after the rendering, to it should not slow it.

Good point, so this should be reviewed to be included in a future release.

@turb
Copy link
Author

turb commented Apr 23, 2018

Good point, so this should be reviewed to be included in a future release.

@oparoz thanks, any idea if it can be done before next release?

@oparoz
Copy link
Member

oparoz commented Apr 23, 2018

We need people to test and review this PR. There is still 2 months to make it in the next release. 🤞

@turb
Copy link
Author

turb commented Sep 10, 2018

@oparoz do you think it may be possible having someone review this one?

@MorrisJobke
Copy link
Member

If the description is too long then not everything is shown. Maybe instead of replacing the name just showing it via the tooltip that is shipped in Nextcloud is the better approach (see for example the tooltips on the elements in the files sidebar).

I used the "long_description.jpg" from https://github.com/ianare/exif-samples/tree/master/jpg

@MorrisJobke MorrisJobke added the 2. developing Work in progress label Oct 3, 2018
@nickvergessen
Copy link
Member

The gallery app has been replaced by the beautiful new app:
Nextcloud Photos - 📸 Your memories under your control

Please checkout if your Pull request is still necessary there, and in case create it there or raise an issue for others to copy the change from here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
2. developing Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants