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

Add height and width to image file data #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bendik
Copy link

@bendik bendik commented Jun 1, 2018

Maybe this could be hidden in some opts for the general public that don't require variable crop ratio layout calculation on server render. ;)

@jondashkyle
Copy link
Owner

Super solid stuff. Had thought about this and held off implementing as I’d like to retain feature parity between Node’s fs and the Dat WebArchive API exposed in Beaker Browser. Perhaps checking for the necessary methods to run sizeOf and executing it conditionally is the right call. @jongacnik @s3ththompson

@bendik
Copy link
Author

bendik commented Jun 6, 2018

I was imagining this working fine if fs and WebArchive readFile(Sync) both returned a buffer type and beaker had similar buffer API's, but WebArchive gives me an ArrayBuffer.

If reading all the images into a buffer is unnecessary load on beaker – which seems likely – what is the best conditional to put next to parsedFile.type === 'image' to make this feature Node exclusive? (!!fs.statSync maybe)

I might continue to dig a little further into using the ArrayBuffer from the WebArchive API to extract the information needed to do these calculations, just for kicks.

@s3ththompson
Copy link

I ran into this issue before with different tradeoffs between server-side (Node) and browser-side (Beaker/web). I think it might be nice to have a standard convention for adding small bits of functionality/metadata where it makes sense on the Node side. I like the idea of checking whether fs is defined.

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.

3 participants