search

reconhub / learn

RECON learn: a free, open platform for training material on epidemics analysis
https://reconlearn.org
Other
35 stars 30 forks source link

Stegen #44

Closed thibautjombart closed 5 years ago

thibautjombart commented 5 years ago

This is not ready to merge yet, but a first complete version of the Stegen practical. Doing a PR to facilitate deployment and keep conversation going here.

thibautjombart commented 5 years ago

The code collapsing does not work when the document is embedded in the website, but works when compiled separately using rmarkdown::render() - output attached

practical-stegen.html.zip

zkamvar commented 5 years ago

@thibautjombart, since you are down in the Bijou, do you want me to just go ahead and make the suggested changes?

thibautjombart commented 5 years ago

Reviewing this now with my morning coffee :)

thibautjombart commented 5 years ago

Thanks for the super thorough review! I agree with all changes but one, the use of pkg::function syntax. I would think it is crucial when teaching package development, but less so in an introduction. The ratio of how much questions this will raise vs actual practical usefulness is not worth it, in my opinion. Would be good to ask Pat or Alex what they think?

Anyway, super cool. Can you implement the changes and merge, and then submit to review to Amrish?

thibautjombart commented 5 years ago

I forgot to mention: if 'case study' in types does not create a dedicated page, let's leave it as 'practical' then, so it gets listed there.

zkamvar commented 5 years ago

Regarding the buttons:

There are a couple of issues that I don't currently have the expertise to solve:

  1. using hugo shortcodes to be able to render markdown within the collapsible div tag
  2. paring down the bootstrap css to isolate just the classes we want (btn, btn-success, and collapsible). Alex has created a portable collapse class, but we don't have the buttons.

I recently just merged #41, which fixed the width of the code blocks and shoved the details tags over to the left side, so I'm going to go back to using those for now, update the suggestions I made, and then I'll futz with including the buttons in a separate PR.

zkamvar commented 5 years ago

I'm at a stopping point. I'm pretty sure things need to be a bit more fleshed out but I am le tired and still have to make my presentations and shit before the end of the week. Take what I added or leave it; I don't particularly care.

zkamvar commented 5 years ago

Imma merge this. It will probably need a bit more polishing, but that can be done in a separate PR