Skip to content

updated chart nav. fix #561#581

Merged
jwildfire merged 2 commits intodevfrom
chartNavFormatting
Jun 16, 2021
Merged

updated chart nav. fix #561#581
jwildfire merged 2 commits intodevfrom
chartNavFormatting

Conversation

@jwildfire
Copy link
Contributor

@jwildfire jwildfire commented Jun 15, 2021

Summary

Updates chart nav formatting using makeChartSummary(). Fixes #561.

Test notes

Please review in conjunction with SafetyGraphics/safetyCharts#67. Confirm that chart nav is updated to include chart type and domain, and that all charts are running as expected after version updates. Use update-configs branch in safetyCharts along with this branch for safetyGraphics as shown below:

# Run from Github
devtools::install_github("safetyGraphics/safetyCharts", ref="update-configs")
library(safetyCharts)
devtools::install_github("safetyGraphics/safetyGraphics", ref="chartNavFormatting")
library(safetyGraphics)
safetyGraphics::safetyGraphicsApp()

@xni7
Copy link
Contributor

xni7 commented Jun 15, 2021

Nice job! Had a couple of minor observations

  • Minor suggestion on the chart order
    • demographics
    • AE charts: ae explorer, timeline, tendril
    • lab charts
  • Chart export was not successful - server error. Seems to be caused by multi-domain charts: AE explorer and Tendril. Single domain charts seem to work.
Warning in if (chart$domain == "multiple") { :
  the condition has length > 1 and only the first element will be used
aeExplorer has an init.
Quitting from lines 71-106 (report.Rmd) 

Warning: Error in UseMethod: no applicable method for 'select' applied to an object of class "NULL"
  • When using devtools::install_github("safetyGraphics/safetyCharts", ref="update-configs") RStudio IDE prompted updating R packages including Shiny and bslib to latest on github. The developmental version broke the app
bslib            0.2.5.9002  2021-06-15 [1] Github (rstudio/bslib@2a6e62d)  
shiny          * 1.6.0.9021  2021-06-15 [1] Github (rstudio/shiny@b3247d5)

See err msg. Downgrading to Shiny 1.6.0 on CRAN made the err go away. Perhaps in the vignette we instruct users to update CRAN pkgs only?

Warning: Error in : 'tab_append' is not an exported object from 'namespace:bslib'
  55: %>%
  54: module
  49: callModule
  48: server
Error : 'tab_append' is not an exported object from 'namespace:bslib'
In addition: Warning messages:
1: package ‘dplyr’ was built under R version 4.0.4 
2: Navigation containers expect a collection of `bslib::nav()`/`shiny::tabPanel()`s and/or `bslib::nav_menu()`/`shiny::navbarMenu()`s. Consider using `header` or `footer` if you wish to place content above (or below) every panel's contents.

@jwildfire
Copy link
Contributor Author

Thanks @xni7. I triaged those comments in to issues. #582 is definitely a bug. I'll update the order in safetyCharts real fast now. Anything else of note? If not, let's go ahead and merge these, and deal with the other comments in separate PRs.

@xni7
Copy link
Contributor

xni7 commented Jun 16, 2021

Thanks @xni7. I triaged those comments in to issues. #582 is definitely a bug. I'll update the order in safetyCharts real fast now. Anything else of note? If not, let's go ahead and merge these, and deal with the other comments in separate PRs.

yes, agreed, let's merge

@jwildfire
Copy link
Contributor Author

@xni7 Want to put in a quick review to mark this approved? I updated the chart order per your suggestion.
image

@xni7
Copy link
Contributor

xni7 commented Jun 16, 2021

The new chart ordering looks good!

@jwildfire jwildfire merged commit 2aba129 into dev Jun 16, 2021
@jwildfire jwildfire deleted the chartNavFormatting branch June 17, 2021 15:05
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.

Enforce consistent chart names in dropdown

2 participants