Skip to content
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

Error in rendering links on instructor view #404

Closed
brownsarahm opened this issue Mar 6, 2023 · 2 comments · Fixed by #409
Closed

Error in rendering links on instructor view #404

brownsarahm opened this issue Mar 6, 2023 · 2 comments · Fixed by #409
Assignees
Labels
bug Something isn't working

Comments

@brownsarahm
Copy link
Contributor

When we include images, we can use the relative path and sandpaper turns it into the correct path after building: https://preview.carpentries.org/instructor-training/fig/mental_models.svg,

but when we have links in an instructor note, it turns relative paths into:

https://preview.carpentries.org/instructor-training/instructor/fig/array-math.png

when the correct path should be:

https://preview.carpentries.org/instructor-training/fig/array-math.png

this is the underlying cause of: carpentries/instructor-training#1501

@zkamvar zkamvar added the bug Something isn't working label Mar 7, 2023
@zkamvar zkamvar self-assigned this Mar 7, 2023
@zkamvar
Copy link
Contributor

zkamvar commented Mar 11, 2023

The reason why this was not caught is because instructor pages live inside of the instructor/ subfolder, which only copies the HTML pages, not the assets. Images are accurately rendered because we have a function that takes the images from the HTML and adds a backtrace to the path:

sandpaper/R/utils-xml.R

Lines 115 to 123 in 4eebe10

use_instructor <- function(nodes = NULL) {
if (length(nodes) == 0) return(nodes)
copy <- xml2::read_html(as.character(nodes))
# lnk <- xml2::xml_find_all(copy, ".//a[not(starts-with(@href, 'http'))]")
img <- xml2::xml_find_all(copy, ".//img[not(starts-with(@src, 'http'))]")
# xml2::xml_set_attr(lnk, "href", fs::path("instructor/", xml2::xml_attr(lnk, "href")))
xml2::xml_set_attr(img, "src", fs::path("../", xml2::xml_attr(img, "src")))
as.character(copy)
}

However, I did not go further with internal links because that ended up always taking people outside of instructor mode, but I could explicitly filter for relative links that are not HTML.

@zkamvar
Copy link
Contributor

zkamvar commented Mar 11, 2023

And yes, I am embarrased by the dead code inside that function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants