-
Notifications
You must be signed in to change notification settings - Fork 89
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
Adding height and width attributes to img tags #146
Comments
note that this puts extra strain on helix-static. reading the dimensions can be expensive, especially when the image is large. it is important to only download enough to read the image headers.... alternatively, we could store the dimensions in the blob metadata and retrieve them via a HEAD request. the image metadata could be added during after the image is uploaded to azure. as long as the meta data is missing, we would deliver the image tag w/o dimensions. note, that the |
Regarding determining image size: we are using image-size here. It's very popular and lightweight. IIRC it's not always possible to determine the dimension from the first 1k bytes... |
Great points.
We can also consider creating a new
At first I thought this would only be useful for google and word, but we can just upload images from GitHub, too. |
I don't think this would work, I'd be surprised if the "expansion" library we use to generate the HTML tags supports esi tags… but might be worth a try |
I rather though of a background process that scans the newly uploaded blobs...this could also help with the qr-code processing for larger images. also, there was once the idea of uploading all the images to azure (see adobe/helix-static#88) |
FWIW: a while ago I wrote an Azure Blob Triggger that deals with QR code decoding: https://github.com/stefan-guggisberg/azure-blob-trigger |
and why don't we use this :-) ? |
someone voiced concerns regarding using cloud platform specific APIs IIRC... |
I wonder who that was :-) In hindsight, it might be a good solution to avoid overloading the runtime actions during document processing and also allow for async extraction of the non-document originating blobs. |
yes, I'd reopen adobe/helix-static#88 if we start with asynchronous asset processing, would it make sense to process with NUI? |
one more thought: I had a concern that async processing might lead to slow delivery or to caching responses without size info, but there is a good way around this: until the size info is available, deliver with short-lived cache headers, afterward with long lived headers. |
Wouldn't that be overkill for our simple use case (determining size and QR decoding)? I'd prefer Azure Blob Triggers, dead simple and way more efficient (they run on storage). |
ok. we just must not miss the point when two simple things become a dozen complex things. |
after thinking about this for a while, i think we should definitely support this. while there are not a lot cases where this actually matters for i think the implementation would rely on the extraction of the height and width of embedded images at the time of extraction (eg. word2md and gdoc2md) and possibly close to the point where the content addressable hash is calculated, we would parse the relevant parts of the jpg/gif/png headers to extract the height and the width. |
There is a CommonMark proposal that has been implemented in ![](file.jpg){ width=50% } The proposal hasn't found consensus and there is no remark-plugin either. The full-to-spec alternative would be a plain |
I think we rather just want to transport the width/height, but not use it. so maybe encode it in the fragment like we do with the type:
or
or
although the last one is probably not desired either. |
I like your second option best. |
Overview
People on the internet say it's a good idea for performance.
There are two problems:
Details
A possible opt-in solution:
helix-markup.yaml
configureimg
tags to be replaced withesi:include
tags that use (@ramboz: is this possible?) a.img
extension to point tohelix-static
helix-static
fetches the image and renders height and width (in the future it may do even more like srcsets and DAM-powered art direction)Proposed Actions
and now?
The text was updated successfully, but these errors were encountered: