finish off plotly implementation

nhs-r-community / NHSRplotthedots

An SPC package to support NHSE/I 'Making Data Count' programme

https://nhs-r-community.github.io/NHSRplotthedots/

Other

48 stars 23 forks source link

finish off plotly implementation #176

Closed tomjemmett closed 1 year ago

tomjemmett commented 1 year ago

finishes off the plotly implementation by adding in unit tests and sorting out check issues

resolves #126

[x] added unit tests and checked code coverage with covr::report() (should aim for 100%)
[x] ran devtools::document()
[x] ran lintr::lint_package() and resolved all lint warnings and notes
[x] ran styler::style_pkg() to make sure code matches the style guidelines
[x] ran R-CMD CHECK and resolved all issues

tomjemmett commented 1 year ago

@marcosfabietti would be good to get a check from you that the plotly implementation is still working! :-)

I've also pushed some changes to the actions as they are out of date

marcosfabietti commented 1 year ago

tested:

with no target
with target
with less than 12 points
changing icon position

everything looks good!

tomjemmett commented 1 year ago

I've just noticed there is a bug with icons and facetted plots - all of the icons get inserted in the top right of the plot, not the facet.

Thinking quickly about facets and icons, I can't really see a way that this could ever look sensible (the size of the icons would quickly start dominating the entire plot area). So, I suggest if the plots are facetted then we set icon position to "none" and give a warning if it wasn't already "none"

marcosfabietti commented 1 year ago

tested it and you are right! removing it is the way to go