You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Follow-up PR after closing #14 - Fixes color palette, Metadata page redesign, plotly conversion of remaining static plots.
Changes
Three unified palette constants (SCALE_COLORS, PHENOTYPE_COLORS, META_COLORS). Susceptible is intentionally grey so Resistant contrasts.
Metadata tile redesign: quickStatBox adds an icon_name param; makeQuickStats is now 3×3 tiles with muted colors and icons.
Metadata plots → plotly: makeDatAvailabilityPlot, makeTimeSeriesAMRPlot, makeHostIsolatePlot, makeIsolationSourcesPlot wrapped with ggplotly + per-mark hover text. Same fonts across all four (theme_bw, 10pt axis, 12/10pt legend). Phenotype plots use PHENOTYPE_COLORS; host + source use META_COLORS. Matching UI/server updates in R/app.R.
Dynamic Isolation/Hosts header: header above the bottom-right tabBox on the Metadata page now swaps title text when the active tab changes between 'Hosts' or 'Isolation Sources'.
**Model Performance plot rebuilt with native plot_ly.
Regenerated NAMESPACE and DESCRIPTION with devtools::document().
Test plan
devtools::load_all() then launchAMRDashboard().
Metadata tab:
3×3 redesigned tiles with icons; Susceptible tile is grey.
Data availability + Time series plots: Resistant amber, Susceptible grey, all interactive with hover tooltips.
Overall this looks good. Just two minor things before merging
For META_COLORS in utils.R, there are 11 defined colors. How did you decide on 11 and is it guaranteed there won't be more than 11 hosts/isolation sources/etc? Sfl works fine but if other bacteria can have more then this probably needs to be changed.
roxygen2 options can now be set using Config/roxygen2/ fields in DESCRIPTION (e.g. Config/roxygen2/markdown: TRUE)
instead of the Roxygen field. The old Roxygen field is still supported.
Similarly, the roxygen2 version is now stored in Config/roxygen2/version.
roxygen2 will migrate this automatically the next time you document.
So basically we should only need the Config line now as it migrated for us earlier
Note also I noticed this (2) in PR #25 as well
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up PR after closing #14 - Fixes color palette, Metadata page redesign, plotly conversion of remaining static plots.
Changes
SCALE_COLORS,PHENOTYPE_COLORS,META_COLORS). Susceptible is intentionally grey so Resistant contrasts.quickStatBoxadds anicon_nameparam;makeQuickStatsis now 3×3 tiles with muted colors and icons.makeDatAvailabilityPlot,makeTimeSeriesAMRPlot,makeHostIsolatePlot,makeIsolationSourcesPlotwrapped withggplotly+ per-mark hover text. Same fonts across all four (theme_bw, 10pt axis, 12/10pt legend). Phenotype plots usePHENOTYPE_COLORS; host + source useMETA_COLORS. Matching UI/server updates inR/app.R.plot_ly.Regenerated
NAMESPACEandDESCRIPTIONwithdevtools::document().Test plan
devtools::load_all()thenlaunchAMRDashboard().Checklist
Please ensure that all boxes are checked before indicating that this pull request is ready for review.