Shrav dev2021

[shravanisamala]

Changed the many style guide issues that were present. Not sure if the layout for the real data analysis works so please let me know if you have any better layout ideas!

[kqp5476]

Everything works smoothly;
I like hoe the icons on the navigation buttons corresponded to the page of the destination - I'll definitely be using that in my updates for the future;
I would add a period after your name in the "Acknowledgements" section

[NurulSyafiqah-pixel]

Just a minor spelling mistake of Prof. in "Special thanks to Pro. Neil for coding help" in the overview page under the acknowledgment section.

[JuddHe]

the spelling mistake has been fixed, please help me to review the app again. thank you, Junjie

…

On Thu, Jul 7, 2022 at 4:33 PM Nurul Syafiqah Mohammad Hamdi < ***@***.***> wrote: ***@***.**** requested changes on this pull request. Just a minor spelling mistake of Prof. in "Special thanks to Pro. Neil for coding help" in the overview page under the acknowledgment section. — Reply to this email directly, view it on GitHub <#5 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AZFNJFZ2SUSN3AY3II37WTDVS45LLANCNFSM46MCLHWQ> . You are receiving this because your review was requested.Message ID: ***@***.***>

[Xinyue-Tang]

I think this app is really good!

[neilhatfield]

This app needed some major work and you have done a good job so far. There are still some issues that need to be addressed. Some of these may require my direct assistance, so please don't hesitate to reach out to me to schedule a time to work.

Files

• Delete any files that are either old/out-of-date and/or no longer used. Examples include 2016Count.PNG, Feature.css from the www folder.
• Double check whether helper.r and model.txt are actually being used. If they aren't, delete them.

Description

• Fix the title; there's an extra "- a lengthy title" that isn't needed
• Set the date to 2019-08-01
• Add real data to the tags list
• add "for odds ratios" to the end of the second learning outcome
• Set the lifecycle to experimental

README

• Change the lifecycle badge to experimental (see Lifecycle Badges in the Style Guide)
• Let's find a better screenshot; we typically aim for something that shows action

app.R

There is a lot here but I will help you.

• Let's remove any unused packages from the library list; ultimately, I would like to remove shinyjs and markdown from the list
• Any code that has been commented out should be examined for why.
• Any code that is no longer being used (commented or not) should be deleted.
• Change "dashboard" to "tachometer-alt" for the overview page
• Use the new "book-reader" icon for the example page.
• Remove "About:" from the first paragraph of the overview page.
• Any time you see style = "text-align: center" add a semi-colon (;) just after "center" (i.e., style = "text-align: center;")
• If you want to leave me in as special thanks, write my name as Neil Hatfield.
• Check on the wording throughout the Pre-req's page. Make sure that all sentences make sense.
• The tables on the pre-req's page need to be actual tables; not pictures. Since they are static examples, we can write them directly in the UI. I can help with this.
• The notation used in the pre-req's page needs to be explained. What do the symbols mean?
• For the mathematics in the pre-req's page, we can use \[ and \] to create centered math without needing to use the style = "text-align: center;" arguments they currently use.
• There are several broken popovers on the explore page. There is code for popovers but no appear when I test the app.
• Replace all actionButton with bsButton; a few arguments related to size might need to change as a result.
• Remove all of the extra buttons that move the user from page to page
• We don't use sidebarLayout any more. We prefer to use fluidRow, with column and wellPanel so that we have better control over the layout. I can assist with how to do the conversion.
• The Example page needs more text to help the user understand what this is an example of and importantly, how to read the plot.
• There are a lot of conditionalPanels on the Example page. Let's replace these with ui elements which get updated by the server instead.
• Some of the examples have a comment which can be shown/hidden. Let's make these comments always appear. I find that they are extremely helpful.
• The plots appear to be picture files instead of actual plots. Why?
• There are several instances of HTML(markdownToHTML(...)). These should be replace with regular text.
• Update all references to be the most current.
• Fix the info button (no colon)
• I found some elements in the server that I don't see where they go. For example, there's output$hypo and input$testdesigncheckbox. Please investigate these (and any others) and prepare recommendations on what we should do with them.
• Throughout the app there are either missing or inappropriate alt text on images. We need to fix these.
• Go through the server code section carefully. Try to document this code as much as possible. Feel free to leave comments such as "I don't understand what this code does but it links to the ___[ object ]___ on the ____ page."

General Coding Issues

• Remove any extra blank lines in the code. (You may keep one blank line between blocks of unrelated code.)
• Remove any unnecessary colons.
• Any instance of tags$style([contents]) should be deleted along with their contents. (This is interfering with our central CSS for the book.
• Look for other places where the style argument has been used. Generally, the only allowed instance is style = "text-align: center;". If you see something like style = "font-size: 18px", these instances need to be deleted. (Conflicts with central CSS.)
• There are inappropriate usages of headings, in particular h4.
• We want to be explicit and name all arguments in functions. Be sure to do so.
• Be sure that the code is formatted appropriately in terms of spacing throughout.

[Sburke26]

Here are some issues I noticed while reviewing:

The first badge does not match the Lifecycle value in the DESCRIPTION file.
The year badge does not properly match the year that the app was created.
The button on the overview page is not labelled according to where it sends the user.
The alt arguments are not assigned meaningful text.
Some object names are not meaningful or do not use camelCase properly.
There is a warning message in the console: "Warning: Using size aesthetic for lines was deprecated in ggplot2 3.4.0."
When minimizing the window of the app on the explore page, the "Sample Odds Ratio" text overlaps the "Sample Percentage" text.

Other small details I noticed that may be improved:

The use of images instead of rendering plots in the Example page makes the values difficult to read sometimes.
The prerequisites page is a bit messy. It may be worth looking into making it more attractive for the user.
Some of the code carries over the 80 characters margin without needing to.

[robchapp]

Adding on to Sean's review, the headings in the examples are not capitalized, along with the button in the overview not correct with the icon and label. Otherwise Sean listed other issues.