-
Notifications
You must be signed in to change notification settings - Fork 4
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: image asset ids #107
fix: image asset ids #107
Changes from all commits
5b0a285
b64f475
0a535a0
c49d642
8d05157
cf4ceb5
73f736b
215a857
becf822
ec6dd82
1bc5558
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@dotlottie/dotlottie-js": patch | ||
--- | ||
|
||
fixes issues with wrong images ids |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -221,11 +221,12 @@ export class DotLottieCommon { | |
* @param newName - desired id and fileName, | ||
* @param imageId - The id of the LottieImage to rename | ||
*/ | ||
private _renameImage(animation: LottieAnimationCommon, newName: string, imageId: string): void { | ||
animation.imageAssets.forEach((imageAsset) => { | ||
private async _renameImage(animation: LottieAnimationCommon, newName: string, imageId: string): Promise<void> { | ||
for (const imageAsset of animation.imageAssets) { | ||
if (imageAsset.id === imageId) { | ||
// Rename the LottieImage | ||
imageAsset.renameImage(newName); | ||
const oldPath = imageAsset.fileName; | ||
|
||
await imageAsset.renameImage(newName); | ||
|
||
if (!animation.data) throw createError('No animation data available.'); | ||
|
||
|
@@ -236,27 +237,42 @@ export class DotLottieCommon { | |
// Find the image asset inside the animation data and rename its path | ||
for (const asset of animationAssets) { | ||
if ('w' in asset && 'h' in asset) { | ||
if (asset.id === imageId) { | ||
if (asset.p === oldPath) { | ||
asset.p = imageAsset.fileName; | ||
} | ||
} | ||
} | ||
} | ||
}); | ||
} | ||
} | ||
|
||
private _renameImageAssets(): void { | ||
const images: Map<string, LottieImageCommon[]> = new Map(); | ||
/** | ||
* Generates a map of duplicate image ids and their count. | ||
* @returns Map of duplicate image ids and their count. | ||
*/ | ||
private _generateMapOfOccurencesFromImageIds(): Map<string, number> { | ||
const dupeMap = new Map<string, number>(); | ||
|
||
this.animations.forEach((animation) => { | ||
images.set(animation.id, animation.imageAssets); | ||
animation.imageAssets.forEach((imageAsset) => { | ||
if (dupeMap.has(imageAsset.id)) { | ||
const count = dupeMap.get(imageAsset.id) ?? 0; | ||
|
||
dupeMap.set(imageAsset.id, count + 1); | ||
} else { | ||
dupeMap.set(imageAsset.id, 1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to be an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was duplicateMap as duplicate filename not the content of the image, will change to filename occurence map |
||
} | ||
}); | ||
}); | ||
|
||
let size = 0; | ||
return dupeMap; | ||
} | ||
|
||
images.forEach((value) => { | ||
size += value.length; | ||
}); | ||
/** | ||
* Renames the image assets in all animations to avoid conflicts. | ||
*/ | ||
private async _renameImageAssets(): Promise<void> { | ||
const dupeMap = this._generateMapOfOccurencesFromImageIds(); | ||
|
||
for (let i = this.animations.length - 1; i >= 0; i -= 1) { | ||
const animation = this.animations.at(i); | ||
|
@@ -266,8 +282,18 @@ export class DotLottieCommon { | |
const image = animation.imageAssets.at(j); | ||
|
||
if (image) { | ||
this._renameImage(animation, `image_${size}`, image.id); | ||
size -= 1; | ||
let count = dupeMap.get(image.id) ?? 0; | ||
|
||
if (count > 0) { | ||
count -= 1; | ||
} | ||
|
||
dupeMap.set(image.id, count); | ||
|
||
if (count > 0) { | ||
// Rename the image | ||
await this._renameImage(animation, `${image.id}_${count}`, image.id); | ||
} | ||
} | ||
} | ||
} | ||
|
@@ -509,7 +535,7 @@ export class DotLottieCommon { | |
|
||
if (this.animations.length > 1) { | ||
// Rename assets incrementally if there are multiple animations | ||
this._renameImageAssets(); | ||
await this._renameImageAssets(); | ||
this._renameAudioAssets(); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
id
is not always the same as thefilename
, for exampleid:
image_0
, filename:image_0.png
is not the same as
id:
image_0
, filename: 'image_0.webp'right ? probably our ids should be the filename and we can get rid of using the image id in this case, no ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inside the lottie itself, images had an id field, and then a name and path attribute, thats why I mapped it like this to the image class.
The id is just the filename without the extension
In your example the id stayed the same despite changing the extension