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

Exif and audio panels #10956

Merged
merged 10 commits into from
Jun 6, 2024
Merged

Exif and audio panels #10956

merged 10 commits into from
Jun 6, 2024

Conversation

kulmann
Copy link
Member

@kulmann kulmann commented May 23, 2024

Description

This PR introduces an EXIF panel, which displays image, photo and location metadata if that's available on the given resource as well as an Audio Info panel, which displays audio metadata if that's available on the given resource.

Related Issue

Motivation and Context

Provide useful additional information.

Screenshots

Screenshot 2024-05-23 at 16 16 12 Screenshot 2024-05-24 at 14 57 39

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests
  • Documentation
  • Maintenance (e.g. dependency updates or tooling)

Open tasks:

Copy link

update-docs bot commented May 23, 2024

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@kulmann kulmann changed the title Exif panel Exif and audio panels May 24, 2024
@kulmann kulmann force-pushed the exif-panel branch 2 times, most recently from 0a20175 to f2d81fa Compare May 24, 2024 13:05
@AlexAndBear
Copy link
Contributor

AlexAndBear commented May 24, 2024

Would love to change "Musical genre" to "Genre". Sounds a little too lofty.

@kulmann
Copy link
Member Author

kulmann commented May 24, 2024

Would love to change "Musical genre" to "Genre". Sounds a little too lofty.

I was getting inspiration from the field labels in mac finder file info. :-) But also fine with Genre, changed it 👍

@kulmann kulmann force-pushed the exif-panel branch 2 times, most recently from 091eaa1 to 747ee01 Compare May 29, 2024 08:16
@kulmann kulmann force-pushed the exif-panel branch 2 times, most recently from 4b1ec54 to 2d66bfd Compare June 4, 2024 08:50
@@ -67,6 +69,7 @@ export default defineComponent({
return '-'
}
if ([5, 6, 7, 8].includes(unref(resource).photo?.orientation)) {
// these orientations indicate portrait mode. tika normalizes width and height according to orientation.
Copy link
Member

Choose a reason for hiding this comment

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

What a weird thing to do, should we maybe handle this on server side instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

please not for this PR 🙈 maybe as a followup 😅

Copy link
Member

Choose a reason for hiding this comment

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

sure sure...

@kulmann kulmann marked this pull request as ready for review June 5, 2024 14:16
@kulmann kulmann requested a review from dschmidt June 5, 2024 14:24
if (!data) {
return data
}
const converted = {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be typed, otherwise the return of the method is not properly typed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't have a better type than Record<string, any> here 🙈

kulmann and others added 2 commits June 5, 2024 21:35
…anel.vue

Co-authored-by: Jannik Stehle <50302941+JammingBen@users.noreply.github.com>
@kulmann kulmann requested a review from JammingBen June 5, 2024 19:59
Copy link

sonarcloud bot commented Jun 5, 2024

<dt v-text="$gettext('Dimensions')" />
<dd data-testid="exif-panel-dimensions" v-text="dimensions" />
<dt v-text="$gettext('Device make')" />
<dd data-testid="exif-panel-cameraMake" v-text="cameraMake" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have the mix aus Kebap and camelCase here? 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

Because it's only data-testids? 🙈 Normally we use kebab case in html. I'm using them in a generic way and just inline the object keys. The object keys are camelCase. If you're super unhappy with that I can transform the object keys to kebab case first. 🤷

Copy link
Contributor

Choose a reason for hiding this comment

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

fine for me, is wa just confused

@AlexAndBear AlexAndBear self-requested a review June 6, 2024 07:44
@kulmann kulmann requested a review from dschmidt June 6, 2024 07:45
Copy link
Contributor

@AlexAndBear AlexAndBear left a comment

Choose a reason for hiding this comment

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

🚀 Wonderful, hope to do some of the calculations in the backend later so its easier for other clients to implement

@kulmann kulmann merged commit 4247277 into master Jun 6, 2024
3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the exif-panel branch June 6, 2024 07:54
ownclouders pushed a commit that referenced this pull request Jun 6, 2024
* feat: exif panel for image and photo metadata and audio panel
@dschmidt
Copy link
Member

dschmidt commented Jun 6, 2024

🎉 🎉 🎉

I love it so so much! Thanks!

Now ... multiple root panels? ;-)

@micbar micbar mentioned this pull request Jun 19, 2024
24 tasks
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