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.

Sign up for GitHub

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

feat(web): Show lens model in the asset viewer detail panel #15460

Merged
merged 3 commits into from
Jan 23, 2025
Merged

feat(web): Show lens model in the asset viewer detail panel #15460

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
96961d6
Adds lens details to the asset viewer
bxtdvd Jan 20, 2025
e97fdb8
Update lens detail search links
bxtdvd Jan 23, 2025
13d76c7
Merge branch 'main' of github.com:immich-app/immich into lens-details
alextran1502 Jan 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions i18n/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,7 @@
"latest_version": "Latest Version",
"latitude": "Latitude",
"leave": "Leave",
"lens_model": "Lens model",
"let_others_respond": "Let others respond",
"level": "Level",
"library": "Library",
Expand Down Expand Up @@ -1113,7 +1114,9 @@
"search_camera_model": "Search camera model...",
"search_city": "Search city...",
"search_country": "Search country...",
"search_for_camera": "Search for this camera",
"search_for_existing_person": "Search for existing person",
"search_for_lens": "Search for this lens",
"search_no_people": "No people",
"search_no_people_named": "No people named \"{name}\"",
"search_options": "Search options",
Expand Down
41 changes: 40 additions & 1 deletion web/src/lib/components/asset-viewer/detail-panel.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,21 @@

const toggleAssetPath = () => (showAssetPath = !showAssetPath);

const getCameraSearchHref = (asset: AssetResponseDto) => {
const cameraSearchUrl = new URL(AppRoute.SEARCH, globalThis.location.href);
cameraSearchUrl.searchParams.set(
QueryParameter.QUERY,
`{"make":"${asset.exifInfo?.make}","model":"${asset.exifInfo?.model}"}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

This'll not work if the make or model have double quotes in them. It's probably best to use JSON.stringify for the entire value. Same concerns with getLensModelSearchHref.

I also think this'll give you unexpected results if the make is unset or if the model is unset, since converting undefined to a string gives you the string literal "undefined".

Additionally, you could probably deduplicate the two get*Href functions further since everything but the value parameter of the set call is the same.

Does a helper function already exist to be able to specify the search URL? Quick digging reveals getMetadataSearchQuery, so I imagine you'd want to use that and/or whatever code currently calls this function.

);
return cameraSearchUrl.href;
};

const getLensModelSearchHref = (asset: AssetResponseDto) => {
const lensSearchUrl = new URL(AppRoute.SEARCH, globalThis.location.href);
lensSearchUrl.searchParams.set(QueryParameter.QUERY, `{"lensModel" : "${asset.exifInfo?.lensModel}"}`);
return lensSearchUrl.href;
};

let isShowChangeDate = $state(false);

async function handleConfirmChangeDate(dateTimeOriginal: string) {
Expand Down Expand Up @@ -410,7 +425,31 @@
<div><Icon path={mdiCameraIris} size="24" /></div>

<div>
<p>{asset.exifInfo.make || ''} {asset.exifInfo.model || ''}</p>
<p>
<a
href={getCameraSearchHref(asset)}
title={$t('search_for_camera')}
class="hover:dark:text-immich-dark-primary hover:text-immich-primary"
>
{asset.exifInfo.make || ''}
{asset.exifInfo.model || ''}
</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

If neither the make nor model are set, will this still render the a? Seems like you'd want to do the check higher up, similar to line 439.

</p>

{#if asset.exifInfo?.lensModel}
<div class="flex gap-2 text-sm">
<p>
<a
href={getLensModelSearchHref(asset)}
title={$t('search_for_lens')}
class="hover:dark:text-immich-dark-primary hover:text-immich-primary"
>
{asset.exifInfo.lensModel || ''}
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think this logical OR is needed because of the if on 439.

</a>
</p>
</div>
{/if}

<div class="flex gap-2 text-sm">
{#if asset.exifInfo?.fNumber}
<p>ƒ/{asset.exifInfo.fNumber.toLocaleString($locale)}</p>
Expand Down
1 change: 1 addition & 0 deletions web/src/routes/(user)/search/[[photos=photos]]/[[assetId=id]]/+page.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@
state: $t('state'),
make: $t('camera_brand'),
model: $t('camera_model'),
lensModel: $t('lens_model'),
personIds: $t('people'),
originalFileName: $t('file_name'),
};
Expand Down
Loading