Make header "Let's Talk" CTA button green

[davidrsch]

What changed

The "Let's Talk" actionLink in the top-right of the header (app/view/mod_header.R, class cta-button) now renders with a green background instead of the brand blue.

• app/styles/_header.scss — .header .right .cta-button background: var(--primary) → background: var(--green).
• app/static/css/app.min.css — the same single declaration updated in the committed Sass build artifact, so the change takes effect without requiring a local rhino::build_sass().

Why

Requested via shinybot: change the header "Let's Talk" button to green through its CSS styling.

Notes on the approach

• --green (#3a5a40, defined under branding.colors in config.yml and injected as a CSS variable by generate_css_variables()) was reused rather than hardcoding a hex value, keeping the button themeable like the rest of the app. The existing color: white label is left as-is — white on #3a5a40 is comfortably above WCAG AA contrast.
• app/static/css/app.min.css was edited surgically (one declaration) rather than regenerated. The committed artifact is already stale with respect to the Sass sources — it contains a .logs-llm-placeholder img rule that no longer exists in app/styles/, and it was produced by a libsass-era compiler whose output formatting differs from current dart-sass. A full rebuild would therefore have buried this one-line change in a whole-file reformat plus unrelated rule removals. CI runs rhino::build_sass() on push, which regenerates the file from the SCSS source in any case.

How it was tested

• Compiled app/styles/main.scss with dart-sass to confirm the SCSS is valid and emits .cta-button{background:var(--green);...}.
• The R test suite (rhino::test_r()) and Rhino linters could not be run in this environment: rhino, shiny, config, box and the rest of the renv library are not installed. The change touches only CSS declarations and no R code, and the existing tests (tests/testthat/, plus a Cypress smoke test that only asserts the app starts) do not assert on button colors.

---

🤖 Opened by the shinybot-bridge mediforce workflow, on behalf of a shinybot chat request.

[davidrsch]

🤖 Automated review by shinybot-bridge — flag false positives so we can tune the prompt.

Summary

Two CSS declarations changed, both saying the same thing: .header .right .cta-button background var(--primary) → var(--green), in app/styles/_header.scss and mirrored into the committed app/static/css/app.min.css. Matches the request exactly — no scope creep, no R code, no CI/secrets/auth touched.

Verified independently, not taken from the PR body:

• --green exists and is injected. config.yml → branding.colors.green: "#3a5a40"; generate_css_variables() (app/logic/general_utils.R:51) emits it into :root. No hardcoded hex — consistent with the theming approach.
• The hand-edited build artifact is correct. Compiled app/styles/main.scss with dart-sass and diffed rule-by-rule against the committed app.min.css: the only deltas are cosmetic minifier differences (.75em vs 0.75em, #fff vs white, rgba(0,0,0,0) vs transparent) plus one pre-existing stale rule. The edited declaration is byte-identical in meaning to what a real rhino::build_sass() emits. Since rhino.yml sets sass: node, the committed artifact is what the app serves — editing it was required, not optional.
• No regression on the replaced value. .cta-button has exactly one consumer (app/view/mod_header.R:44) and one rule; nothing else depends on the CTA being primary-coloured.
• Contrast improves. White on #3a5a40 = 7.73:1 (AAA). The old --primary #0099f9 was 3.03:1 — below AA for normal text. Leaving color: white was the right call.

Findings

Minor

• app/styles/_header.scss:33 — The CTA now shares the log-status success colour. --green is a semantic status token: app/styles/_colors.scss:5 uses the same variable for .green-text, which marks OK/success log entries. The CTA and the status palette are now coupled — retuning status green for legibility in the log list will silently restyle the header button. A dedicated branding.colors.cta token would decouple them. Deliberately not changed here: adding a token for a one-line chat request is scope the requester didn't ask for. Question for the reviewer: is sharing the status token acceptable, or should the CTA get its own colour?
• app/styles/_header.scss:33 — "green" was under-specified and #3a5a40 is a dark, muted forest green, not a bright CTA green. It is the only green in the palette, so reusing it is the defensible reading, but the visual result may not be what the requester pictured. Worth an eyeball on the deployed preview before merge.
• PR description — the CI claim is wrong in a way that could mislead a future change. The body says rhino::build_sass() on push "regenerates the file from the SCSS source in any case". .github/workflows/rhino-test.yml does run build_sass(), but the output stays in the runner and is discarded — there is no commit-back and no check that the committed artifact matches the source. Hand-editing app.min.css was load-bearing, not a convenience. The code is right; only the justification is off, and the next person who trusts it will ship stale CSS.

Nit

• app/static/css/app.min.css:1 — pre-existing artifact drift, verified not introduced here: .logs-llm-placeholder img{width:50%} entered in 86b1d69 ("feat: add llm styling") and the matching SCSS was removed later without a rebuild, so the artifact has outlived its source. This diff touches the file but should not fix it — a rebuild would bury a one-line change under a whole-file reformat. Worth a separate PR that regenerates the artifact and, better, a CI step that fails when the committed CSS and the SCSS disagree.

Test coverage

No test asserts the button colour, and none is requested here. tests/cypress/e2e/app.cy.js only smoke-tests startup; tests/testthat/ covers utils. Adding a visual-regression harness for a two-declaration CSS change would be disproportionate. Noting the gap, not asking for it. The implementer could not run rhino::test_r() or the linters (renv library absent) — acceptable, since no R or JS changed; CI will run lint_sass() on push, and the SCSS compiles cleanly.

Security

Nothing to flag. No secrets, credentials, workflow files, or auth paths touched; no user input reaches the change; the diff is two CSS declarations.

Verdict

APPROVE — no blockers or majors. The change does exactly what was asked, and both files are consistent. Before merge: confirm #3a5a40 is the green you wanted, and consider correcting the CI claim in the PR body so it does not become precedent.

[davidrsch]

This a hackathon test