Skip to content

Unify color palette + redesign Metadata page + plotly conversion#24

Open
eboyer221 wants to merge 2 commits into
mainfrom
dev-color-palette-unification
Open

Unify color palette + redesign Metadata page + plotly conversion#24
eboyer221 wants to merge 2 commits into
mainfrom
dev-color-palette-unification

Conversation

@eboyer221
Copy link
Copy Markdown
Contributor

@eboyer221 eboyer221 commented May 28, 2026

Description

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

  1. devtools::load_all() then launchAMRDashboard().
  2. 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.
    • Host + Isolation source plots: muted categorical palette, interactive.
    • Choropleth: amber gradient, interactive.
    • Sankey: muted palette, Resistant node = amber, Susceptible node = grey.
    • Click between "Isolation sources" and "Hosts" sub-tabs updates header title accordingly.
  3. Model Performance tab: three boxes side-by-side at each species position, drug-overlay dots land on their matching box.

Checklist

Please ensure that all boxes are checked before indicating that this pull request is ready for review.

  • I have read and followed the CONTRIBUTING.md guidelines.
  • I have searched for existing content to ensure this is not a duplicate.
  • I have performed a self-review of these additions (including spelling, grammar, and related).
  • I have added comments to my code to help provide understanding.
  • I have added a test which covers the code changes found within this PR.
  • I have deleted all non-relevant text in this pull request template.
  • Reviewer assignment: Tag a relevant team member to review and approve the changes.

@amcim
Copy link
Copy Markdown
Contributor

amcim commented May 28, 2026

Overall this looks good. Just two minor things before merging

  1. 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.

  2. The DESCRIPTION has this line added RoxygenNote: 7.3.3 but PR Package name change from amRshiny to amRviz + New Visualizations #20 added Config/roxygen2/version: 8.0.0 because roxygen got an update this month and we regenerated documentation there

From the roxygen release notes (https://roxygen2.r-lib.org/news/index.html):

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

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.

2 participants