Open elifiner opened 1 year ago
Thanks for this PR. This sounds like a very useful transformation.
Can I ask you to write some unit tests for the new method that you've written. This would self document the context that you've just shared in this PR and would help me in making changes with confidence in the future. Also, need to make sure that we handle exception scenarios (such as nulls or missing images).
Will do. Should I add a cover image to the default book template as well? [image: vcs]
On Mon, Apr 3, 2023 at 2:44 PM Hady Osman @.***> wrote:
Thanks for this PR. This sounds like a very useful transformation.
Can I ask you to write some unit tests for the new method that you've written. This would self document the context that you've just shared in this PR and would help me in making changes with confidence in the future. Also, need to make sure that we handle exception scenarios (such as nulls or missing images).
— Reply to this email directly, view it on GitHub https://github.com/hadynz/obsidian-kindle-plugin/pull/246#issuecomment-1495023905, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGHNNN2AY34QNSPVFNRXJTW7NADVANCNFSM6AAAAAAWR2U47I . You are receiving this because you authored the thread.Message ID: @.***>
I think that would be a good idea. We've had several questions (https://github.com/hadynz/obsidian-kindle-plugin/issues/231 https://github.com/hadynz/obsidian-kindle-plugin/issues/56) pertinent to book covers so it would be nice for it to be part of the default template
Why not fetch the full resolution image and/or let users decide what dimensions to use?
Currently returned by scraper: https://m.media-amazon.com/images/I/91wTP3k8ZtL._SY160.jpg Available in max resolution: https://m.media-amazon.com/images/I/91wTP3k8ZtL.jpg
You could also do it in the template itself by replacing the height value part of the URL:
{% if imageUrl -%}
![{{title}} book cover|150]({{imageUrl | replace("._SY160", "")}})
{% endif -%}
which returns
![Elantris book cover|150](https://m.media-amazon.com/images/I/91wTP3k8ZtL.jpg)
This is all going off of the assumption that the full-resolution image is even available, I wonder if it would be a better idea to return a separate metadata variable with the high-res link, under the assumption that it may not be available.
Scraped | Without Height constraint |
---|---|
![]() |
![]() |
I wanted to add covers to my book summaries but using
{{imageUrl}}
in the template resulted in tiny, barely legible, images.The default URL pulled from Amazon are for images that are 160px in height.
Added a regex to change
imageUrl
to pull images that are 1024px in width.If you want to render the image in the template, you can use the following markdown format (replace 320 with whatever width you want the image to render):
{% if imageUrl %}![|320]({{imageUrl}}){% endif %}
Considered adding this to the default template, but this seems to belong in a different PR.