-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
Callout body sometimes looks like the callout title #470
Comments
I'm afraid I couldn't recreate this on my side, when I was building locally to preview your changes in datacarpentry/openrefine-socialsci#154, @bencomp. All of the boxes looked okay to me. Further investigation is warranted, I think. |
Thanks for checking, @tobyhodges – let me check if I can get more debugging output, or at least see what versions of package I have installed. I can already tell that the same happens when I build the current main branch and that it doesn't matter if I use So this could very well be related to my installation. And because I did install with Homebrew, I understand if it is out of scope. Packages installed in R
Some of the Brewed dependencies (I don't know which are most relevant):
|
This should have been fixed in {sandpaper} 0.12.0 via #467 and appears to be correct in the live version of the site which was built an hour ago: I'm not sure what's going on with your installation, locally though. My suggestion would be to rebuild the site from scratch to see if you are running into a cache issue: sandpaper::build_lesson(rebuild = TRUE) For context, this bug is partially because when we were designing the front end, we did not have an example of a callout block embedded within another callout block (with the exception of the :::::::::::::::::::::::::::::::::::::::::: prereq
## Software
For this lesson you will need **OpenRefine** (formerly Google Refine) and a
web browser. Basic installation steps are provided on this page.
The OpenRefine [installation manual](https://openrefine.org/docs/manual/installing)
provides more details about installation, upgrades and configuration.
Note: this is a Java program that runs on your machine (not in the cloud).
It runs inside your browser, but no web connection is needed for this lesson.
:::::::::::::::::::::::::::::::::::::: callout
### Administrator rights
You do not need administrative rights on the computer to *install* OpenRefine.
However, if anti-malware software blocks OpenRefine when you try to start it,
you may need administrative rights to allow OpenRefine to *run*.
OpenRefine is safe to run.
:::::::::::::::::::::::::::::::::::::::::::::::
:::::::::::::::::::::::::::::::::::::::::::::::::: This is also why the styling for the callout block looks like a prereq block. |
Also, for future, you can give me your session information using the built in
|
Thanks for these function pointers and comments! Perhaps my versions are a tiny bit behind yours (downloaded from the Netherlands mirror, which could be behind?), although I also have packages that you don't. Looking at https://github.com/carpentries/sandpaper/pull/467/files#diff-b6d0e1001a0e8491b46773f0256ae272453ba38c9f2e666d4f7ce49197c8068dR11-R16 (from #467) I see that this I will see if it continues to occur. |
I think the only way you would have seen this with version 0.12.0 is if you had the development version installed, but even then, I think I only switched it to version 0.12.0 after I fixed the callout blocks anchor IDs (which was an impetus for me to bump the minor version), so it's a bit of a mystery. A slow mirror wouldn't affect that because {sandpaper} only comes from The Carpentries R Universe, not CRAN. That being said, considering it's no longer a problem on the deployed site, I will close this for now. Please reopen if this is still happening. |
I'm reopening this because I've discovered that the the source of this issue is due to a change in behaviour between pandoc 2 and 3 in carpentries/workbench#59 (comment): lsn <- usethis::create_from_github("zkamvar/test-workbench-59", destdir = tempdir())
#> ℹ Defaulting to 'https' Git protocol
#> ✔ Setting `fork = FALSE`
#> ✔ Creating '/tmp/Rtmp24N1j4/test-workbench-59/'
#> ✔ Cloning repo from 'https://github.com/zkamvar/test-workbench-59.git' into '/tmp/Rtmp24N1j4/test-workbench-59'
#> ✔ Setting active project to '/tmp/Rtmp24N1j4/test-workbench-59'
#> ℹ Default branch is 'main'
#> ✔ Setting active project to '<no active project>'
pandoc::pandoc_activate("3.1.3")
#> ✔ Version '3.1.3' is now the active one.
#> ℹ Pandoc version also activated for rmarkdown functions.
sandpaper::build_lesson(lsn, quiet = TRUE, preview = FALSE)
idx <- xml2::read_html(fs::path(lsn, "site/docs/index.html"))
xml2::xml_find_first(idx, ".//div[starts-with(@class, 'callout')]") |> as.character() |> writeLines()
#> <div id="callout1" class="callout">
#> <div class="callout-square">
#> <i class="callout-icon" data-feather="bell"></i>
#> </div>
#> <div class="section level3 callout-title callout-inner">
#> <h3 class="callout-title" id="being-certain-which-system-your-terminal-is-connected-to">Being certain which system your terminal is
#> connected to<a class="anchor" aria-label="anchor" href="#being-certain-which-system-your-terminal-is-connected-to"></a>
#> </h3>
#> <div class="callout-content">
#> <p>If you ever need to be certain which system a terminal you are using
#> is connected to then use the following command:
#> <code>$ hostname</code>.</p>
#> </div>
#> </div>
#> </div> Created on 2023-07-21 with reprex v2.0.2 I believe it's coming from this part of the lua filter where we place the callout header (which has the "callout-title" class) inside of the "callout-inner" block: sandpaper/inst/rmarkdown/lua/lesson.lua Line 278 in 4b2a219
|
A bit more background for clarification: pandoc operates in this way: flowchart TB
input -->|"reader (+[options])"| AST
AST -->|filter| AST
AST -->|"writer (--[options])"| output
The reason why we are seeing the difference in behaviour is because the HTML-writer-specific option The thing is, we already correct for this here and I think the issue is that we are searching for "callout ", when pandoc 3 collapses duplicate classes, so I think the solution is to search that the div starts with callout Lines 98 to 100 in 4b2a219
|
You're probably on to something, @zkamvar! A change in Pandoc across major versions would explain such changes in output. If I read it correctly, Would a workaround be to adjust the CSS selector from |
This indeed would work and it would be a good idea to implement that. It's important to note that only "callout" blocks (but not "discussion", "keypoints" or any other blocks) are affected because pandoc 3 (rightfully) deduplicates classes, thus my selection of
Yes. It's been a topic of some debate in pandoc for the past few years: jgm/pandoc#5965, which is probably why I've never gone for the |
Hi, Just wanted to chip in and say I'm seeing this bug when building lessons (I'm currently converting https://github.com/carpentries-incubator/snakemake-novice-bioinformatics from the old format). I've installed all the Sandpaper dependencies, including Pandoc and R, in a fresh Conda environment on Ubuntu. In my case, every single callout has this problem where The quick fix was to force a downgrade of Pandoc in the conda env:
It might be worth noting this on the requirements document (https://carpentries.github.io/sandpaper-docs/#required) as Pandoc 2.x does seem to be the preferred version just now and is explicitly used in the GitHub actions. |
Apologies as I'm coming fairly new into the Workbench and its maintenance, and my R is very much lapsed! From my reading through and checking the instructor-training lesson as per Nathaniel's report, the only callouts affected by this are the
In the former, the resulting div has only one class,
This seems to be the reason that the existing Is there a reason why we couldn't use an additional XPath selector as:
In addition to using the current starts-with? https://github.com/carpentries/sandpaper/blob/eb009faca249bac5b999ac5449990329d3eb7d0d/R/utils-xml.R#L200C41-L200C83 Using the specific class selector seems to grab the correct callout parent divs, and would require no other changes to other code as far as I can tell? I've tested this with pandoc 2.19.2 and 3.11.1 and both seem to produce the same "clean" callout-inner class structure despite the pandoc3 behaviour. |
After a bit more testing this morning, it seems:
Produces consistent results across pandoc 2.x and 3.x. I'll raise a PR. |
shorten and rename episode 2 @chennesy going to merge this, but on the "Variables Persist Between Cells" everything is underlined. I looked but didn't see what was causing it.
This should now be resolved in #574 so please do test and let us know of any other issues. |
Can this issue be closed @froggleston? |
I don't know why it happens, but sometimes the CSS class
callout-title
is added to the<h3>
element (correctly) and its parent<div>
element (incorrectly). When that happens, the body of the callout inherits the CSS directives for the title:(This is from my local build; I see no difference before and after updating to the latest sandpaper.)
I also don't see an immediate pattern – it doesn't happen with every callout or only within nested callouts.
The files that show this error are in datacarpentry/openrefine-socialsci#154, if anyone would like to try to reproduce this issue.
The text was updated successfully, but these errors were encountered: