Moved thumbnail image tag to Block with conditional lazy load. #95

rho_n · 2023-03-11T15:40:30Z

rho_n commented

2023-03-11 15:40:30 +00:00

Add lazy loading of host avatars to the correspondents page

rho_n commented

2023-03-11 15:41:43 +00:00

@gordons I created a work in progress pull request to make code review easier

Thanks for your help, I do hope you have time and inclination to keep contributing.

In general the template file naming scheme is (I will admit sometimes I get lazy and don't follow my own rules):

content-*.tpl.html : main HTML/TT2 macros for an individual page
navigation-*.tpl.html : the menu template used by an individual page
queries-*.tpl.html : the database specific queries used by an individual page
ids-*.tpl.html : generates a list of ids for the site-generator for multi-page templates (i.e. correspondent and host templates)
rss-*.tpl.xml : templates for generating RSS feeds
shared-*.tpl.html : these are "library" templates used by multiple pages, or in multiple spots within a template to minimize repeating HTML/TT2 macros. Multiple macros can live in the same file.

Your lazy image loading code is good and think it should be moved to a lazy_load macro and placed in shared-avatar.tpl.html file. We can use it in the future for lazy loading other images.

The macro get_avatar should be renamed get_avatar_src, and move your host_thumb macro host-thumbnail.tpl.htm to the shared-avatar.tpl.html and update it to use the new lazy_load macro.

Update pages to call just the get_avatar macro.

@gordons I created a work in progress pull request to make code review easier Thanks for your help, I do hope you have time and inclination to keep contributing. In general the template file naming scheme is (I will admit sometimes I get lazy and don't follow my own rules): * content-*.tpl.html : main HTML/TT2 macros for an individual page * navigation-*.tpl.html : the menu template used by an individual page * queries-*.tpl.html : the database specific queries used by an individual page * ids-*.tpl.html : generates a list of ids for the site-generator for multi-page templates (i.e. correspondent and host templates) * rss-*.tpl.xml : templates for generating RSS feeds * shared-*.tpl.html : these are "library" templates used by multiple pages, or in multiple spots within a template to minimize repeating HTML/TT2 macros. Multiple macros can live in the same file. Your lazy image loading code is good and think it should be moved to a lazy_load macro and placed in shared-avatar.tpl.html file. We can use it in the future for lazy loading other images. The macro get_avatar should be renamed get_avatar_src, and move your host_thumb macro host-thumbnail.tpl.htm to the shared-avatar.tpl.html and update it to use the new lazy_load macro. Update pages to call just the get_avatar macro.

gordons commented

2023-03-12 08:06:30 +00:00

I've renamed get_avatar to get_avatar_src in all the places.
Moved host_thumb into shared-avatar.tpl.html and added a parameter for when after the host_cnt lazy loading should be added.
I'm a bit confused about the other things you wrote.

Separately, I have a way of adding lazy loading to the images in each show notes, but I need to make it work in template land first. This will be in a different issue and performed in a different way to these host avatars, since it will be processing HTML in notes variable.

I don't think a generic lazy_load macro is terribly practical for this site without a lot of parameters for attributes.

I will try my best to follow your naming conventions etc. The hard tabs threw me for a second. I'm not partically great at naming variables.

I've renamed **get_avatar** to **get_avatar_src** in all the places. Moved **host_thumb** into `shared-avatar.tpl.html` and added a parameter for when after the **host_cnt** lazy loading should be added. I'm a bit confused about the other things you wrote. Separately, I have a way of adding lazy loading to the images in each show notes, but I need to make it work in _template land_ first. This will be in a different _issue_ and performed in a different way to these host avatars, since it will be processing HTML in `notes` variable. I don't think a generic **lazy_load** macro is terribly practical for this site without a lot of parameters for attributes. I will try my best to follow your naming conventions etc. The hard tabs threw me for a second. I'm not partically great at naming variables.

rho_n commented

2023-03-14 03:23:06 +00:00

@gordons I was going to try and explain my "vision" for the name changes and combining macro's, but just updating the code was quicker and clearer :)

Let me know if you are ok with the changes, or have any suggestions or corrections. If none, feel free to remove the WIP: from the title of this PR and merge it with main.

Thanks!

@gordons I was going to try and explain my "vision" for the name changes and combining macro's, but just updating the code was quicker and clearer :) Let me know if you are ok with the changes, or have any suggestions or corrections. If none, feel free to remove the WIP: from the title of this PR and merge it with main. Thanks!

gordons commented

2023-03-16 14:03:37 +00:00

Not sure content-episode.tpl.html should have show_avatar instead of get_avatar because I'm not sure it had a link before, but it might be better that it does. Ditto with content-twat_episode.tpl.html I guess.

Other than that it all looks good and makes sence.

Not sure `content-episode.tpl.html` should have **show_avatar** instead of **get_avatar** because I'm not sure it had a link before, but it might be better that it does. Ditto with `content-twat_episode.tpl.html` I guess. Other than that it all looks good and makes sence.

rho_n commented

2023-03-16 16:11:56 +00:00

Not sure content-episode.tpl.html should have show_avatar instead of get_avatar because I'm not sure it had a link before, but it might be better that it does. Ditto with content-twat_episode.tpl.html I guess.

I double checked the HPR website and there are links on the main website. I did also see closing anchor tags without the opening tag, which is what made me add it back. I must have deleted accidentally along the way. Going to merge into main.

> Not sure `content-episode.tpl.html` should have **show_avatar** instead of **get_avatar** because I'm not sure it had a link before, but it might be better that it does. Ditto with `content-twat_episode.tpl.html` I guess. I double checked the HPR website and there are links on the main website. I did also see closing anchor tags without the opening tag, which is what made me add it back. I must have deleted accidentally along the way. Going to merge into main.

Sign in to join this conversation.

No reviewers