Skip to content

Conversation

@gadenbuie
Copy link
Member

@gadenbuie gadenbuie commented Nov 18, 2020

question() accepts HTML for the text argument, in particular using htmltools tags. But the default value of the loading argument uses c() to concatenate c("Loading...", text), which results in an unusual list that quiz_text() isn't expecting and markdown::renderMarkdown() throws an error.

question(
  htmltools::tags$p("Did this work?"),
  answer("yes"),
  answer("no", correct = TRUE)
)

## Error in renderMarkdown(file, output = NULL, text, renderer = "HTML",  : 
##   argument "file" is missing, with no default

I just changed the default to c("Loading", format(text)) so that the shiny tags are formatted as a character.

PR task list:

  • Update NEWS
  • Add tests (if possible)
  • Update documentation with devtools::document()

I'll add a NEWS item if the PR looks good to you.

@schloerke
Copy link
Collaborator

LGTM!

@gadenbuie
Copy link
Member Author

gadenbuie commented Nov 23, 2020

I found another related case where a non-character answer() text causes issues

question(
  text = "1+1",
  answer(2, correct =TRUE)
)
## Error in renderMarkdown(file, output = NULL, text, renderer = "HTML",  : 
##   argument "file" is missing, with no default

and I added https://github.com/gadenbuie/learnr/blob/d94855cfd92327273909472e9c5e2f457d5fe73e/R/quiz.R#L227-L229 in quiz_text() before the text is handed off to markdown::markdownToHTML().

@gadenbuie gadenbuie requested a review from schloerke December 30, 2020 18:36
gadenbuie added a commit to gadenbuie/learnr that referenced this pull request Dec 30, 2020
@gadenbuie gadenbuie mentioned this pull request Dec 30, 2020
4 tasks
@schloerke schloerke merged commit 9399a95 into rstudio:master Jan 4, 2021
@gadenbuie gadenbuie deleted the fix-loading-text-html branch January 19, 2021 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants