rho_n
/
hpr_generator
4
2
Fork 1
Code Issues 34 Pull Requests Projects 1 Releases Wiki Activity

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

Merged
rho_n merged 9 commits from I90_Lazy_load_host_avatars into main 2023-03-16 16:13:49 +00:00
Conversation 5 Commits 9 Files Changed 8 +21 -20
rho_n commented 2023-03-11 15:40:30 +00:00
Owner
Copy Link

Add lazy loading of host avatars to the correspondents page

Add lazy loading of host avatars to the correspondents page
rho_n added the
Feature Request
 label 2023-03-11 15:40:30 +00:00
gordons was assigned by rho_n 2023-03-11 15:40:30 +00:00
rho_n added 1 commit 2023-03-11 15:40:31 +00:00
rho_n commented 2023-03-11 15:41:43 +00:00
Author
Owner
Copy Link

@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 added 1 commit 2023-03-12 07:49:36 +00:00
ce4aefe828 Renamed macro get_avatar to get_avatar_src as per suggestion. 
Moved host_thumb macro into shared avatar template file.
Made condition of lazy loading a parameter.
gordons commented 2023-03-12 08:06:30 +00:00
Collaborator
Copy Link

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 added 6 commits 2023-03-14 03:17:17 +00:00
5b4421071f
Add MACRO lazy_load 
Macro which allows programatically setting the HTML5 img
loading attribute.
dee924e6a8
Rename MACRO host_thumb to get_avatar 
Update to use lazy_load MACRO and minimize number of
arguments needed.
rho_n added 1 commit 2023-03-14 03:20:37 +00:00
rho_n commented 2023-03-14 03:23:06 +00:00
Author
Owner
Copy Link

@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
Collaborator
Copy Link

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 changed title from WIP: Moved thumbnail image tag to Block with conditional lazy load. to Moved thumbnail image tag to Block with conditional lazy load. 2023-03-16 16:10:16 +00:00
rho_n commented 2023-03-16 16:11:56 +00:00
Author
Owner
Copy Link

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.
rho_n closed this pull request 2023-03-16 16:12:13 +00:00
rho_n reopened this pull request 2023-03-16 16:13:26 +00:00
rho_n merged commit 860ce671f8 into main 2023-03-16 16:13:49 +00:00
rho_n deleted branch I90_Lazy_load_host_avatars 2023-03-16 16:14:02 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
Bug Found

For issues related to the code or the content of the created HTML files

  
Feature Request

   
General Discussion

For discussing HPR site generation in general

  
Help Request

For issues running or installing the site-generator

No Label
Bug Found
Feature Request
General Discussion
Help Request
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
gordons
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: rho_n/hpr_generator#95
No description provided.
Delete Branch "I90_Lazy_load_host_avatars"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?