Print collection - add missing link/titles for reference-link links - #9489
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces an isPrinting flag to the content rendering options to adjust how reference links are processed during printing. While this allows for specialized link handling in print views, the feedback highlights that the flag is not currently propagated to recursive rendering calls, which will lead to inconsistent behavior in nested content. Additionally, the reviewer noted that modifying the anchor tag directly instead of using an inner span may result in style loss when the print view processes the links.
…r aware of printing)" This reverts commit 62a8fb2.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the postProcessRichContent function to use getAttribute("href") instead of the href property when processing reference links. A review comment suggests optimizing performance by parallelizing the link title loading with Promise.all instead of processing them sequentially in a loop.
| for (const el of referenceLinks) { | ||
| const innerSpan = document.createElement("span"); | ||
| await link.loadReferenceLinkTitle($(innerSpan), el.href); | ||
| await link.loadReferenceLinkTitle($(innerSpan), el.getAttribute("href")); | ||
| el.replaceChildren(innerSpan); | ||
| } |
There was a problem hiding this comment.
Processing reference links sequentially with await inside a loop can be inefficient, especially in large documents or print collections where many links might be present. Since the notes are already prefetched at line 47, these calls will likely be fast, but parallelizing them with Promise.all is a better practice to avoid unnecessary sequential blocking and improve rendering performance.
| for (const el of referenceLinks) { | |
| const innerSpan = document.createElement("span"); | |
| await link.loadReferenceLinkTitle($(innerSpan), el.href); | |
| await link.loadReferenceLinkTitle($(innerSpan), el.getAttribute("href")); | |
| el.replaceChildren(innerSpan); | |
| } | |
| await Promise.all(referenceLinks.toArray().map(async (el) => { | |
| const innerSpan = document.createElement("span"); | |
| await link.loadReferenceLinkTitle($(innerSpan), el.getAttribute("href")); | |
| el.replaceChildren(innerSpan); | |
| })); |
Reference links were not displayed in printed version. Neither links for collection notes, nor note titles for notes outside of the collection.
Note:

Printed version:

This PR fixes that