Filmstrip bugfixes #48
Conversation
|
Just an odd thing I've noticed - not sure what's caused it, but lately there's a 1 px line all the way along the top!
|
|
I've not looked into dark mode in a while. Thank you for catching that -- I'll fix 👍 |
|
I fixed up the thumbnail view to use percent offset rather than pixel offset - resulting in a cleaner preview (should no longer result in showing parts of a subsequent/previous frame) 🎉 |
| * @param fileName -- file name to hash | ||
| * @param fileSize -- file size to hash | ||
| */ | ||
| export function hashFile(fileName: string, fileSize: string): string { |
There was a problem hiding this comment.
This method was simply moved (with no modification) from above - to be closer to the line which calls it 👌
There was a problem hiding this comment.
Just FYI - I'm thinking about slightly tweaking the hash function! 👍
| * @param fileName -- file name to hash | ||
| * @param fileSize -- file size to hash | ||
| */ | ||
| export function hashFile(fileName: string, fileSize: number): string { |
There was a problem hiding this comment.
Simply moved to another location within the same file 👌
whyboris
added
✏️ ready for review
|
I did not see a white stripe in my dark mode ... I'll check from time to time to see if it ever appears for me 🙆♂️ |
|
Found how to reproduce the line - zoom out the app once in dark mode! |
|
✅ Confirmed - can replicate the white line bug: zooming out using the |
| @@ -590,17 +578,28 @@ function extractMetadataForThisONEFile( | |||
| const origWidth = metadata.streams[0].width; | |||
| const origHeight = metadata.streams[0].height; | |||
| const sizeLabel = labelVideo(origWidth, origHeight); | |||
| const fileSize = metadata.format.size; | |||
| const fileSize: string = metadata.format.size; // looks like a number, but actually a string! | |||
There was a problem hiding this comment.
Is this intentional? ie. is there a reason we have it as a string?
There was a problem hiding this comment.
It turns out that ffprobe outputs format.size as a string (I checked with console.log(typeOf(metadata.format.size)) -- so I added the type definition to track what happens to it 👍
There was a problem hiding this comment.
Ha! - Should we parse the integer here?
There was a problem hiding this comment.
Makes sense -- will do 👍
| * @param fileName -- file name to hash | ||
| * @param fileSize -- file size to hash | ||
| */ | ||
| export function hashFile(fileName: string, fileSize: string): string { |
There was a problem hiding this comment.
Just FYI - I'm thinking about slightly tweaking the hash function! 👍
|
Sounds good -- feel free to edit the hash as you see fit - no worries about breaking changes as we work before v2.0.0 is released 👍 |
This PR is for minor cleanup and two small features:
There are some lingering issues since the last PR (#44) - but I'm tracking them with this issue: #50
Please feel free to leave comments about other bugs in the current
masterin #50 👍This PR introduces breaking changes -- old
.vhafiles will error out