theme_civilytics() currently paints the Civilytics cream paper (#FAF7F2) as the default plot.background and panel.background (via paper_bg = TRUE). Request: flip the default to a transparent background, keeping the cream as an explicit opt-in.
Why
Figures are most reusable when their background is transparent — they composite cleanly onto report pages, slides, and web backgrounds of any color. A baked-in cream fill forces every downstream consumer to override it.
We hit this in the crdc-arrests white paper: to get transparent figures we had to call theme_civilytics(paper_bg = FALSE)and set a transparent graphics device (dev = "ragg_png", dev.args = list(background = "transparent")), because the cream comes from the theme's plot.background / panel.background fills.
Proposal
Default theme_civilytics() to transparent (plot.background / panel.background / legend.background fill = NA).
Keep cream as an explicit opt-in — e.g. flip paper_bg to default FALSE, or (clearer) add a background = c("transparent", "paper") argument so users enable the branded cream canvas deliberately.
Document that a transparent device background is also needed for fully-transparent PNGs (the theme alone cannot make the device canvas transparent).
Notes
The machinery already exists: theme_civilytics(paper_bg = FALSE) yields fill = NA for plot/panel/legend backgrounds — so this is a default-flip plus (optionally) a friendlier argument name.
Related: the logo helpers (add_logo() / civilytics_logo() / make_logo_grob()) and footer/overlay placement interact with background color; a transparent default avoids surprising cream bands behind composited logos.
## Summary
`theme_civilytics()` currently paints the Civilytics cream paper (`#FAF7F2`) as the default `plot.background` and `panel.background` (via `paper_bg = TRUE`). Request: **flip the default to a transparent background**, keeping the cream as an explicit opt-in.
## Why
Figures are most reusable when their background is transparent — they composite cleanly onto report pages, slides, and web backgrounds of any color. A baked-in cream fill forces every downstream consumer to override it.
We hit this in the `crdc-arrests` white paper: to get transparent figures we had to call `theme_civilytics(paper_bg = FALSE)` **and** set a transparent graphics device (`dev = "ragg_png"`, `dev.args = list(background = "transparent")`), because the cream comes from the theme's `plot.background` / `panel.background` fills.
## Proposal
- Default `theme_civilytics()` to **transparent** (`plot.background` / `panel.background` / `legend.background` fill = `NA`).
- Keep cream as an explicit opt-in — e.g. flip `paper_bg` to default `FALSE`, or (clearer) add a `background = c("transparent", "paper")` argument so users enable the branded cream canvas deliberately.
- Document that a transparent **device** background is also needed for fully-transparent PNGs (the theme alone cannot make the device canvas transparent).
## Notes
- The machinery already exists: `theme_civilytics(paper_bg = FALSE)` yields `fill = NA` for plot/panel/legend backgrounds — so this is a default-flip plus (optionally) a friendlier argument name.
- Related: the logo helpers (`add_logo()` / `civilytics_logo()` / `make_logo_grob()`) and footer/overlay placement interact with background color; a transparent default avoids surprising cream bands behind composited logos.
kodor
was assigned by jared2026-06-17 19:06:09 -04:00
Document transparent device background needed for PNGs: ✗ — not present in roxygen docs
Tests: New tests cover transparent default and paper_bg = TRUE opt-in. Adequate.
CI: Failing with 1 WARNING. Authoritative blocker per review rules. The quarto removal from Suggests (DESCRIPTION) coincides with extensive quarto assets in inst/quarto/ and exported functions in R/quarto.R — likely the WARNING source, but unverified without the full check log.
quarto removal concern: DESCRIPTION line 11-12 still references "Quarto themes" in the package description, and R/quarto.R exports use_civilytics_revealjs(), use_civilytics_theme(), use_civilytics_brand(). Removing quarto from Suggests while these exist is suspicious and likely causing the CI WARNING.
VERDICT: request_changes
Reviewer (round 5) requested changes:
**Checklist from issue:**
1. **Default to transparent** (`plot.background` / `panel.background` / `legend.background` fill = `NA`): ✓
- `paper_bg = FALSE` default at `R/theme.R:107`
- `bg_color <- if (isTRUE(paper_bg)) paper else NA` at line 115
- `plot.background` (line 269) and `panel.background` (line 240) use `bg_color` → `NA` when `paper_bg = FALSE`
- `legend.background` is `element_blank()` (line 213) — already transparent
2. **Cream as explicit opt-in** (`paper_bg = TRUE`): ✓ — documented, tested, examples updated
3. **Document transparent device background needed for PNGs**: ✗ — not present in roxygen docs
**Tests:** New tests cover transparent default and `paper_bg = TRUE` opt-in. Adequate.
**CI:** Failing with 1 WARNING. Authoritative blocker per review rules. The `quarto` removal from Suggests (DESCRIPTION) coincides with extensive quarto assets in `inst/quarto/` and exported functions in `R/quarto.R` — likely the WARNING source, but unverified without the full check log.
**`quarto` removal concern:** DESCRIPTION line 11-12 still references "Quarto themes" in the package description, and `R/quarto.R` exports `use_civilytics_revealjs()`, `use_civilytics_theme()`, `use_civilytics_brand()`. Removing `quarto` from Suggests while these exist is suspicious and likely causing the CI WARNING.
VERDICT: request_changes
Default to transparent (plot.background / panel.background / legend.background fill = NA)
paper_bg = FALSE default ✓ (line 104)
bg_color = NA when paper_bg = FALSE ✓ (line 112)
plot.background / panel.background use bg_color ✓ (lines 237, 266)
legend.background = element_blank() ✓ (line 210)
Missing test: no test asserts legend.background is transparent/blank by default
Cream as explicit opt-in (paper_bg = TRUE)
paper_bg = TRUE fills with cream ✓ (line 112)
Tests added for paper_bg = TRUE ✓ (lines 142-146)
Documentation updated
@param paper_bg docstring flipped ✓ (lines 39-41)
Examples reordered: transparent first, cream opt-in second ✓ (lines 70-78)
Removed old "transparent background for embedding" example ✓
Missing: issue asks to document that a transparent device background is also needed for fully-transparent PNGs — this note does not appear in the roxygen doc
Verdict: Two gaps: (1) no test for legend.background transparency, and (2) the device-transparency note from the proposal is not documented.
VERDICT: request_changes
Reviewer (round 1) requested changes:
**Issue #7 Checklist:**
1. **Default to transparent** (`plot.background` / `panel.background` / `legend.background` fill = `NA`)
- `paper_bg = FALSE` default ✓ (line 104)
- `bg_color = NA` when `paper_bg = FALSE` ✓ (line 112)
- `plot.background` / `panel.background` use `bg_color` ✓ (lines 237, 266)
- `legend.background = element_blank()` ✓ (line 210)
- **Missing test**: no test asserts `legend.background` is transparent/blank by default
2. **Cream as explicit opt-in** (`paper_bg = TRUE`)
- `paper_bg = TRUE` fills with cream ✓ (line 112)
- Tests added for `paper_bg = TRUE` ✓ (lines 142-146)
3. **Documentation updated**
- `@param paper_bg` docstring flipped ✓ (lines 39-41)
- Examples reordered: transparent first, cream opt-in second ✓ (lines 70-78)
- Removed old "transparent background for embedding" example ✓
- **Missing**: issue asks to document that a transparent **device** background is also needed for fully-transparent PNGs — this note does not appear in the roxygen doc
**Verdict:** Two gaps: (1) no test for `legend.background` transparency, and (2) the device-transparency note from the proposal is not documented.
VERDICT: request_changes
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
theme_civilytics()currently paints the Civilytics cream paper (#FAF7F2) as the defaultplot.backgroundandpanel.background(viapaper_bg = TRUE). Request: flip the default to a transparent background, keeping the cream as an explicit opt-in.Why
Figures are most reusable when their background is transparent — they composite cleanly onto report pages, slides, and web backgrounds of any color. A baked-in cream fill forces every downstream consumer to override it.
We hit this in the
crdc-arrestswhite paper: to get transparent figures we had to calltheme_civilytics(paper_bg = FALSE)and set a transparent graphics device (dev = "ragg_png",dev.args = list(background = "transparent")), because the cream comes from the theme'splot.background/panel.backgroundfills.Proposal
theme_civilytics()to transparent (plot.background/panel.background/legend.backgroundfill =NA).paper_bgto defaultFALSE, or (clearer) add abackground = c("transparent", "paper")argument so users enable the branded cream canvas deliberately.Notes
theme_civilytics(paper_bg = FALSE)yieldsfill = NAfor plot/panel/legend backgrounds — so this is a default-flip plus (optionally) a friendlier argument name.add_logo()/civilytics_logo()/make_logo_grob()) and footer/overlay placement interact with background color; a transparent default avoids surprising cream bands behind composited logos.kodor produced no usable change (attempt 1/2); will retry next run.
kodor produced no usable change (opencode exited 0 with no file changes; attempt 2/2). Handing back — reassign me to retry.
Reviewer (round 1) requested changes:
VERDICT: approve
Reviewer (round 2) requested changes:
Checklist from issue:
Default
theme_civilytics()to transparent (plot.background/panel.background/legend.backgroundfill =NA)paper_bg = FALSEdefault atR/theme.R:107✓bg_color <- if (isTRUE(paper_bg)) paper else NAat line 115 ✓plot.backgroundandpanel.backgroundusebg_colorat lines 269, 240 ✓legend.background = element_blank()at line 213 — already transparent (equivalent tofill = NA) ✓Keep cream as explicit opt-in via
paper_bg = FALSEdefault ✓ (line 107)Documentation updated ✓ — roxygen for
paper_bgrewritten (lines 40-44), examples flipped (lines 73-87)Tests — two new tests added: transparent default (lines 136-140) and
paper_bg=TRUEopt-in (lines 142-146). Existingpaper_bg=FALSEtest preserved (line 188). ✓Variant themes —
theme_civilytics_dark,_map,_dark_mapkeeppaper_bg = TRUE;_slide,_slide_mapkeeppaper_bg = FALSE. All consistent. ✓CI failure —
pdflatex not found/ PDF manual build warning is unrelated to this diff. Not a blocker.VERDICT: approve
Reviewer (round 3) requested changes:
VERDICT: approve
Reviewer (round 4) requested changes:
VERDICT: approve
Reviewer (round 5) requested changes:
Checklist from issue:
Default to transparent (
plot.background/panel.background/legend.backgroundfill =NA): ✓paper_bg = FALSEdefault atR/theme.R:107bg_color <- if (isTRUE(paper_bg)) paper else NAat line 115plot.background(line 269) andpanel.background(line 240) usebg_color→NAwhenpaper_bg = FALSElegend.backgroundiselement_blank()(line 213) — already transparentCream as explicit opt-in (
paper_bg = TRUE): ✓ — documented, tested, examples updatedDocument transparent device background needed for PNGs: ✗ — not present in roxygen docs
Tests: New tests cover transparent default and
paper_bg = TRUEopt-in. Adequate.CI: Failing with 1 WARNING. Authoritative blocker per review rules. The
quartoremoval from Suggests (DESCRIPTION) coincides with extensive quarto assets ininst/quarto/and exported functions inR/quarto.R— likely the WARNING source, but unverified without the full check log.quartoremoval concern: DESCRIPTION line 11-12 still references "Quarto themes" in the package description, andR/quarto.Rexportsuse_civilytics_revealjs(),use_civilytics_theme(),use_civilytics_brand(). Removingquartofrom Suggests while these exist is suspicious and likely causing the CI WARNING.VERDICT: request_changes
kodor could not converge in 5 review rounds — rolled back off the nightly branch; needs a human.
Reviewer (round 1) approved, but CI is timeout — holding for green.
Reviewer (round 2) approved, but CI is timeout — holding for green.
Reviewer (round 3) approved, but CI is timeout — holding for green.
Reviewer (round 4) approved, but CI is timeout — holding for green.
Reviewer (round 5) approved, but CI is timeout — holding for green.
kodor could not converge in 5 review rounds — rolled back off the nightly branch; needs a human.
Reviewer (round 1) approved, but CI is timeout — holding for green.
Reviewer (round 2) approved, but CI is timeout — holding for green.
Reviewer (round 3) approved, but CI is timeout — holding for green.
Reviewer (round 4) approved, but CI is timeout — holding for green.
Reviewer (round 5) approved, but CI is timeout — holding for green.
kodor could not converge in 5 review rounds — rolled back off the nightly branch; needs a human.
Reviewer (round 1) requested changes:
Issue #7 Checklist:
Default to transparent (
plot.background/panel.background/legend.backgroundfill =NA)paper_bg = FALSEdefault ✓ (line 104)bg_color = NAwhenpaper_bg = FALSE✓ (line 112)plot.background/panel.backgroundusebg_color✓ (lines 237, 266)legend.background = element_blank()✓ (line 210)legend.backgroundis transparent/blank by defaultCream as explicit opt-in (
paper_bg = TRUE)paper_bg = TRUEfills with cream ✓ (line 112)paper_bg = TRUE✓ (lines 142-146)Documentation updated
@param paper_bgdocstring flipped ✓ (lines 39-41)Verdict: Two gaps: (1) no test for
legend.backgroundtransparency, and (2) the device-transparency note from the proposal is not documented.VERDICT: request_changes
Reviewer (round 2) approved, but CI is timeout — holding for green.
Reviewer (round 3) approved, but CI is timeout — holding for green.
Reviewer (round 4) approved, but CI is timeout — holding for green.
Reviewer (round 5) output was unreadable after a retry — treating as request_changes.
kodor could not converge in 5 review rounds — work parked on
kodor/park-20260704-7; needs a human.