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

fix(web): Uses asset exif info for desc in asset viewer #4249

Merged
merged 2 commits into from
Jan 12, 2024

Conversation

sellnat77
Copy link
Contributor

@sellnat77 sellnat77 commented Sep 27, 2023

Description

  • Attempts to use the current asset's exifInfo.description in the bound description (When it exists)
  • This approach will set a default value for the description if we have it and differs from the previous approach of trying to set the value of the text area

Fixes #4073
Fixes #6244

How Has This Been Tested?

sorry for the buggy PR @waclaw66 Any other test paths you want me to exercise?

  • Upload image -> Add description -> Add to shared album -> Open image as photo owner -> Open shared link as photo owner -> open shared link in incognito tab(anon user) -> Verify photo description is visible across all viewing methods
  • Upload image AFTER testing above ^ -> Verify existing descriptions are maintained and web doesnt crash
  • Add image without description to shared album -> Verify asset detail is correct
  • Modify image description after upload -> Verify description is maintained in standard view + album views

Screenshots (if appropriate):

Left: As Photo Middle: As photo owner through shared link Right: As anon user in incognito
banana_test
Tested the photo detail when people are present to exercise the code within the if statement
with_people
After editing description just to make sure the detail is updated properly
post_edit

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if applicable

@vercel
Copy link

vercel bot commented Sep 27, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
immich ⬜️ Ignored (Inspect) Visit Preview Oct 18, 2023 2:26am

@jrasm91
Copy link
Contributor

jrasm91 commented Sep 27, 2023

Did you test navigating to the next/previous assets, opening and closing the asset in each view?

@sellnat77
Copy link
Contributor Author

Ohhhhh nice catch ty! next/prev doesnt show the description, working on it

@sellnat77 sellnat77 marked this pull request as ready for review October 18, 2023 02:33
@jrasm91
Copy link
Contributor

jrasm91 commented Jan 12, 2024

Stale, closing. Feel free to re-open if you find a way to make this work correctly.

@jrasm91 jrasm91 closed this Jan 12, 2024
@jrasm91
Copy link
Contributor

jrasm91 commented Jan 12, 2024

Oh you know what, I mis-read that you had actually already fixed my comment. I tested this and it does indeed allow for descriptions in shared links.

@jrasm91 jrasm91 reopened this Jan 12, 2024
@jrasm91 jrasm91 enabled auto-merge (squash) January 12, 2024 20:27
@jrasm91 jrasm91 merged commit 19e9908 into immich-app:main Jan 12, 2024
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Photo descriptions missing from shared links [BUG] Image description missing on shared album
2 participants