Visual citation tooltips

[mattwarkentin]

Related to #167

[mattwarkentin]

So I think this draft PR is relatively close to a working implementation. There still seems to be some whitespace/line break/quotations issue with the HTML from namedropR, so we maybe need to sanitize the HTML somehow.

[gadenbuie]

Would you mind adding your test slides under docs/ or tests/manual/? (You have to force add the example slide source to docs/, I have aggressive gitignores in there.)

[mattwarkentin]

Awesome! Just tried it out and it looks great. I will address your comments, but first, in typical fashion, I would like to take this moment to rethink the API before we go too far...

My hope was always that the citation in xaringan would feel exactly like the citation in standard R Markdown, namely that you would include bibliography: ... in the YAML and cite with @key. Right now we are half way there, but we require this new function exite(). Not a big deal, but here is my sales pitch for achieving cohesion with other formats.

Refactor use_excitation() to accept a bib argument, which defaults to the rmarkdown::metadata, same as usual. This function can also accept any styling arguments for namedropR, rather than having to handle that at the citation level. This makes styling and bibliography management cleaner. use_excitation would, by default, read the bib file(s) and create name drops and store them all in one single JSON/script entry accessible on the JS side of things. Basically everything below would now be handled by use_excitation: https://github.com/gadenbuie/xaringanExtra/blob/e1d02ab2058ce263acc2ab984a1ddc5552cc09ae/R/excitation.R#L36-L79

Then, we eliminate excite() altogether, and go back to allowing people to cite with @key. There would be more work on the JS side of things, but perhaps for a better API. We grab all <p> elements and regex the innerHTML/textContent to find @key et al., and replace them with the <span> and tippy.

Cons:

• More up front computational work on the R side by creating name drops for every bib entry right away
• More work/code on the JS side of things to get working (not really a con)

Pros:

• Consistent citation API with standard R Markdown
• Styling and bibliographies handled once globally, which is probably most sensible
• No need for namespace environments etc. for tracking internal items

Thoughts?

[gadenbuie]

That all sounds pretty great! I like that the namedropping would be consolidated into a single function call and it definitely makes it easier to coordinate the html dependency.

My one concern is that users would need to create a reference file specific to the document. When I used to write with citations, I would often just reference my massive zotero references file since it had everything I needed. I think tooling is better now to move references to a paper (or slide) specific bib file, but one advantage of individual excite() calls is that we know for certain that the user wants that citation. Actually - that's also the argument for keeping excite() since it's very intentional.

[mattwarkentin]

My one concern is that users would need to create a reference file specific to the document. When I used to write with citations, I would often just reference my massive zotero references file since it had everything I needed.

Hmm, yes that would be a bottleneck.

My initial thought would be to have use_excitation() do some magic to regex the R Markdown source file (i.e. self) to find all the cite keys and only prepare name drops for those keys. Could use knitr::current_input(dir = TRUE) to get that information at run-time. That way no extra work is performed. It might be a bit off-brand...

EDIT: FWIW, this is a hacky quick code chunk that works...

library(magrittr)

parsermd::parse_rmd(knitr::current_input(TRUE)) %>% 
  parsermd::rmd_select(parsermd::has_type('rmd_markdown')) %>%
  parsermd::as_tibble() %>% 
  tidyr::unnest(ast) %>% 
  dplyr::mutate(ast = as.character(ast)) %>% 
  dplyr::pull(ast) %>% 
  glue::glue_collapse(sep = '\n') %>% 
  stringr::str_extract_all('@[[:alnum:]]+')

[gadenbuie]

yeah, honestly, having implemented the regex/parsing option before, and having so greatly regretted it, I think excite() is a small price to pay. If it comes down to bringing pandoc citation syntax to xaringan, then I think it's xaringan's job to do that.

[mattwarkentin]

Alright, no worries. Just thought it was worth having this discussion now rather than later.

[codecov-commenter]

Codecov Report

Merging #171 (e019b92) into main (a5343ad) will decrease coverage by 5.03%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #171      +/-   ##
==========================================
- Coverage   51.40%   46.36%   -5.04%     
==========================================
  Files          20       21       +1     
  Lines         782      867      +85     
==========================================
  Hits          402      402              
- Misses        380      465      +85     

Impacted Files	Coverage Δ
R/excitation.R	0.00% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a5343ad...e019b92. Read the comment docs.