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

Fix loading placeholder when question text is HTML #450

Merged
merged 4 commits into from
Jan 4, 2021

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 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