pharmaR / riskassessment

Risk Assessment Demo App: https://rinpharma.shinyapps.io/riskassessment
https://pharmar.github.io/riskassessment/
Other
98 stars 24 forks source link

`bm-752` Adds 'About' tab #761

Closed BarbMik closed 2 months ago

BarbMik commented 3 months ago

Closes #752

Hi, I added an 'About' tab containing the original assessment criteria tab, contacts tab, and contributors tab. The descriptions are in the R-raw folder and the new image files in the www folder. The about tab is it's own module called mod_aboutInfo.R . As this is my first PR, feedback and reviews are much appreciated! I may not have time to apply the suggested changes from the reviews, so I'm saving this as a draft.

aclark02-arcus commented 2 months ago

Notes from 4/9

aclark02-arcus commented 2 months ago

@Jeff-Thompson12, Barbara did a great job preparing this branch which contains the new "About page", closing #752. I made some edits to this branch as well to address some of her concerns from earlier this week, plus I added my own touch. However, now the tests are failing and I'm not exactly sure why. Do you mind reviewing this PR and taking a peek at the tests?

aclark02-arcus commented 2 months ago

Oh wow, RE: 566a918 - what a noob. Sorry, I should have figured that out.

aclark02-arcus commented 2 months ago

Hi @Jeff-Thompson12, I pushed one more commit to clean up the IntroJS txt for the new 'About' tab, so this is ready for a "final" review. Is there anything else we can do with this new tab? Is it missing anything? Does the layout make sense?

Jeff-Thompson12 commented 2 months ago

I really like the layout of the content in the new tabs. Think it looks sharp. I'm not so much of a fan of the inherited layout of the now defunct assessment criteria tab.

  1. This is the only tab who's content does not fill the screen. It looks particularly bad for the assessment criteria overview.
  2. I am of the opinion that "Assessment Criteria" should not be the first tab. I think it should land on "Contributors and Companies" first and maybe "Contact" should be the last tab.
  3. Aside: All of our Navigation Bar pages also have tabs. Should we consider making the navigation bar more of a menu where the tabs are moved to menu items? For example "About" would be a menu with these three "tabs" in a dropdown.

Picture to show the layout I'm not a fan of. image width=100%

aclark02-arcus commented 2 months ago

@Jeff-Thompson12, thanks for the feedback!

  1. I agree. That tab did look weird compared to all the others. I went through all the tabs and made them consistent, but in a slightly new way. If hierarchical tabs and output are indented, I feel it's slightly more readable so I made some adjustments to every tab. Pics below! Tell me what you think. I also added some headers to the 'Contacts' and "Contributors & Companies" sub-tabs.
  2. I'm undecided on the default tab for the 'About' tab. I don't think the Assessment Criteria isn't a bad landing spot. I feel like people won't spend much time on the company contributors & contact info... unless they are just browsing for fun.
  3. I think we can leave it as-is for now, but it's certainly worth checking out for the future. The trouble is, I think it would make sense for some tabs, but not all.

Pics for #1

Old: image

New: image


Old: image

New: image


Old: image

New: image

aclark02-arcus commented 2 months ago
  • Table for "Risk Calculation" has different formatting than the other tables (e.g. Maintenance Metrics) in the "Assessment Criteria Overview"

@Jeff-Thompson12, I did everything except for this one. I go ahead and make it left justified, but I think the table was intentionally formatted differently. Maybe we can save it for a future PR?

aclark02-arcus commented 2 months ago

@jthompson-arcus Logos moved into inst/app/www/images/ folder.