Closed avahoffman closed 1 year ago
No broken urls! :tada: Comment updated at 2022-11-14 with changes from 44192286b16f583b5646d3bb9f402b5a6a66f9f2
:warning: spelling errors :warning: There are spelling errors that need to be addressed. Read this guide for more info. Download the errors here. Comment updated at 2022-11-14 with changes from f7624f450f6646ec41aa5a7b0209ca74c4f9fa59
Re-rendered previews from the latest commit:
Updated at 2022-11-14 with changes from 44192286b16f583b5646d3bb9f402b5a6a66f9f2
Awesome, thanks!
Re: persisting settings
AnVIL_module_settings
) was so that they at least aren't cluttering up the R environment with multiple, miscellaneous names, so they won't bump into anything else in your doc.AnVIL_module_settings
at the end of a module (i.e. stick the rm()
inside the child doc)?re: the example divs
padding-left: 100px;
, since there's no picture.The reason I moved the individual settings inside a list (AnVIL_module_settings) was so that they at least aren't cluttering up the R environment with multiple, miscellaneous names, so they won't bump into anything else in your doc.
I agree. I like how you've done this
This does still mean that individual modules in the same doc can bump into each other. I've been wondering if we should automatically clear out AnVIL_module_settings at the end of a module (i.e. stick the rm() inside the child doc)?
This was my original thought. It doesn't work and anvil_module_settings still persists. Maybe because the child environment is separate?
Although, looking at your divs, they look really good! I might actually want to keep the surrounding div when I drop the module into a book, because it gives students a visual boundary around the instructions for a particular task. So perhaps it's not actually a concern if people copy the whole thing, div and all.
That's what I was thinking. If they are left in, no harm no foul.
In that case, maybe rename the class something like "module" or "instructions" or something, and treat it as part of the AnVIL style (i.e. expect that people will use it in their own books)? It might be nice to have a highlighting box that doesn't come with an image. Might also want to set it so that it doesn't have padding-left: 100px;, since there's no picture. Maybe still worth putting some instructions about keeping/removing the div box somewhere
Noted! I can add that. Do you care if I merge #174 into #170 and we work on the same one?
Do you care if I merge my PR and we work on the same one?
@avahoffman Go for it! The only thing I was working on was moving the variables inside the AnVIL_module_settings
list, and I think you've finished that for me in this PR by updating chapter 8 :)
@KatherineCox great! Thanks for the quick review! :)
I've been wondering if we should automatically clear out AnVIL_module_settings at the end of a module (i.e. stick the rm() inside the child doc)?
This was my original thought. It doesn't work and anvil_module_settings still persists. Maybe because the child environment is separate?
Ah, too bad. I wonder if we could reach up from inside the child environment to the parent with <<-
, and set AnVIL_module_settings
to NULL
?
I considered trying to mess around with withr
, but I think the withr
code would need to be outside the child doc, which would make things more confusing for users. If we need to handle clearing the settings in the main document, I think your rm()
solution is best - people are familiar with rm()
.
we can always change it later if we want to get fancy :)
This builds off of @KatherineCox 's work in #170
rm()
at the end of the chunks