Skip to content

Commit 83a1649

Browse files
Copilotyihui
andauthored
Fix dark theme facet subplot borders and update README (#53)
Fix strip_cdn_versions regex - remove erroneous leading slash that prevented version stripping --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: yihui <163582+yihui@users.noreply.github.com> Co-authored-by: Yihui Xie <xie@yihui.name>
1 parent 52612e1 commit 83a1649

6 files changed

Lines changed: 128 additions & 13 deletions

File tree

.github/copilot-instructions.md

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,18 @@ Since gglite generates HTML/JavaScript visualizations, **plots must be tested in
7979
headless browsers** to make sure they can be rendered correctly and produce no
8080
errors in the browser console. The workflow is:
8181
82-
1. **Render to a full HTML page** — both `.Rmd` and `.R` files can be rendered
83-
to `.html` via `litedown::fuse()`. The output is self-contained (all JS/CSS
84-
embedded) because `embed_resources` is enabled in `copilot-setup-steps.yml`.
82+
> **Prefer SVG rendering for headless tests.** Set
83+
> `options(gglite.renderer = 'svg')` before calling `litedown::fuse()` so the
84+
> rendered output contains an `<svg>` element that you can inspect directly in
85+
> the DOM. SVG attributes (fill, stroke, etc.) are directly readable without
86+
> relying on canvas pixel values, making theme and style verification much
87+
> easier. Use `document.querySelectorAll('svg rect[fill]')` or similar to check
88+
> theme properties.
89+
90+
1. **Render to a full HTML page** — set `options(gglite.renderer = 'svg')`,
91+
then render both `.Rmd` and `.R` files to `.html` via `litedown::fuse()`.
92+
The output is self-contained (all JS/CSS embedded) because `embed_resources`
93+
is enabled in `copilot-setup-steps.yml`.
8594
8695
2. **Open with `google-chrome` under Xvfb** and enable remote debugging. Use
8796
`google-chrome`, **not** `chromium` — `chromium` crashes in this
@@ -140,14 +149,19 @@ errors in the browser console. The workflow is:
140149
{"url": "file:///absolute/path/to/foo.html"})
141150
await asyncio.sleep(8) # wait for G2 charts to render
142151
152+
# For SVG renderer: inspect element attributes directly
143153
r = await js(ws, "JSON.stringify({G2: typeof G2 !== 'undefined', "
144-
"c: document.querySelectorAll('canvas').length})")
154+
"svgs: document.querySelectorAll('svg').length})")
145155
print("Status:", r["result"]["value"])
146156
147-
# Hover over a canvas to trigger tooltip
148-
await send(ws, "Input.dispatchMouseEvent",
149-
{"type": "mouseMoved", "x": 400, "y": 300})
150-
await asyncio.sleep(0.8)
157+
# Example: check area fill color for dark theme
158+
r2 = await js(ws, """
159+
var areas = Array.from(document.querySelectorAll('.area'));
160+
JSON.stringify(areas.map(el => ({fill: el.getAttribute('fill'),
161+
stroke: el.getAttribute('stroke')})))
162+
""")
163+
print("Area elements:", r2["result"]["value"])
164+
151165
await shot(ws, "/tmp/screenshot.png")
152166
153167
asyncio.run(main())
@@ -158,6 +172,8 @@ errors in the browser console. The workflow is:
158172
- The chart container element exists in the DOM.
159173
- The G2 chart renders without JavaScript errors (check `console.error`).
160174
- No warnings or errors appear in the browser console.
175+
- For SVG renderer: inspect element attributes (fill, stroke) directly to
176+
verify theme colors.
161177
162178
### Submitting Plot Changes in PRs
163179

DESCRIPTION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
Package: gglite
22
Title: Lightweight Data Visualization via the Grammar of Graphics
3-
Version: 0.0.28
3+
Version: 0.0.29
44
Authors@R: person("Yihui", "Xie", role = c("aut", "cre"), email = "xie@yihui.name",
55
comment = c(ORCID = "0000-0003-0645-5666"))
66
Description: A lightweight R interface to the AntV G2 JavaScript visualization

R/render.R

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,12 +218,38 @@ build_config = function(chart) {
218218

219219
# Theme: merge global option with per-chart theme
220220
theme = modifyList(as.list(getOption('gglite.theme')), as.list(chart$theme))
221-
# For dark themes, auto-set the view background so facet subplots get a dark
222-
# background (G2 uses transparent by default, making axis/grid hard to see).
221+
# For dark themes, auto-set the view background so the chart and facet
222+
# subplots get a dark background (G2 uses transparent by default).
223223
if (isTRUE(theme$type %in% c('dark', 'classicDark')))
224224
theme = modifyList(list(view = list(viewFill = '#141414')), theme)
225225
if (length(theme)) config$theme = theme
226226

227+
# For faceted charts, propagate the resolved theme to each child mark so
228+
# that G2 applies it to every subplot view. Without this, G2's facetRect
229+
# creates child views with no theme, causing them to fall back to the
230+
# default light theme (transparent background, light axis/grid colors).
231+
# Also inject visible border colors on each child via viewStyle (NOT style
232+
# — G2's mark composition moves 'style' to the mark renderer, while
233+
# 'viewStyle' becomes the view's style used for background area rectangles):
234+
# - mainStroke: frame=TRUE in plot.ts hardcodes '#000'; override via
235+
# viewStyle so G2's deepMix({mainStroke:'#000'}, viewStyle) uses ours.
236+
# - viewStroke: @antv/g defaults rect stroke to black; set explicitly
237+
# so the outer subplot outline is visible on the dark background.
238+
if (!is.null(chart$facet) && length(theme) && length(config$children)) {
239+
dark_facet = isTRUE(theme$type %in% c('dark', 'classicDark'))
240+
config$children = lapply(config$children, function(ch) {
241+
if (is.null(ch$theme)) ch = modifyList(ch, list(theme = theme))
242+
if (dark_facet) {
243+
border_col = 'rgba(255,255,255,0.25)'
244+
vsty = ch$viewStyle %||% list()
245+
if (is.null(vsty$mainStroke)) vsty$mainStroke = border_col
246+
if (is.null(vsty$viewStroke)) vsty$viewStroke = border_col
247+
ch$viewStyle = vsty
248+
}
249+
ch
250+
})
251+
}
252+
227253
# Faceting wraps the spec as a facet view
228254
if (!is.null(chart$facet)) {
229255
config$type = chart$facet$type

README.md

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,26 @@
11
# gglite
2-
An R package for data visualization via g2 (based on the Grammar of Graphics)
2+
3+
> **Experimental** — this package is still under active development. The API may
4+
> change and it has been hurting my brain badly. Feedback welcome!
5+
6+
An R package for interactive data visualization using the [AntV
7+
G2](https://g2.antv.antgroup.com/) JavaScript library, with a [Grammar of
8+
Graphics](https://www.springer.com/gp/book/9780387245447)-style API inspired by
9+
ggplot2. The goal is to have Grammar of Graphics *and* interactivity *and* keep
10+
everything lightweight ({gglite} has only one R package dependency).
11+
12+
## Install
13+
14+
Not on CRAN yet, but r-universe.dev is an awesome service in many ways:
15+
16+
``` r
17+
install.packages('gglite', repos = 'https://yihui.r-universe.dev')
18+
```
19+
20+
You can also try the package without installing in the package playground:
21+
<https://pkg.yihui.org/gglite/playground/> (it also works on your mobile
22+
devices, thanks to webR). See the package site (<https://pkg.yihui.org/gglite/>)
23+
for examples and documentation.
24+
25+
R Markdown, [litedown](#0), Quarto, Jupyter, Shiny, and standalone HTML previews
26+
(in the browser) are all supported.

tests/normalize-notebook.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
def strip_cdn_versions(s: str) -> str:
1414
"""Remove version specifiers (e.g. @5, @v0.14.34) from CDN URLs in src/href attributes."""
15-
return re.sub(r'((src|href)="https://[^"]*?)/@v?\d+(?:\.\d+)*', r'\1', s)
15+
return re.sub(r'((src|href)="https://[^"]*?)@v?\d+(?:\.\d+)*', r'\1', s)
1616

1717

1818
def normalize_value(v):

tests/testit/test-theme.R

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,52 @@ assert('light theme does not set viewFill', {
3737
config = build_config(chart)
3838
(is.null(config$theme$view))
3939
})
40+
41+
assert('dark theme propagates to facet children', {
42+
chart = g2(mtcars, hp ~ mpg | cyl) |> theme_dark()
43+
config = build_config(chart)
44+
(config$children[[1]]$theme$type %==% 'dark')
45+
(config$children[[1]]$theme$view$viewFill %==% '#141414')
46+
})
47+
48+
assert('classicDark theme propagates to facet children', {
49+
chart = g2(iris, Sepal.Length ~ Sepal.Width | Species) |> theme_classic_dark()
50+
config = build_config(chart)
51+
(config$children[[1]]$theme$type %==% 'classicDark')
52+
(config$children[[1]]$theme$view$viewFill %==% '#141414')
53+
})
54+
55+
assert('light theme does not propagate viewFill to facet children', {
56+
chart = g2(iris, Sepal.Length ~ Sepal.Width | Species) |> theme_light()
57+
config = build_config(chart)
58+
(is.null(config$children[[1]]$theme$view))
59+
})
60+
61+
assert('dark theme injects border colors on facet children', {
62+
chart = g2(mtcars, hp ~ mpg | cyl) |> theme_dark()
63+
config = build_config(chart)
64+
(config$children[[1]]$viewStyle$mainStroke %==% 'rgba(255,255,255,0.25)')
65+
(config$children[[1]]$viewStyle$viewStroke %==% 'rgba(255,255,255,0.25)')
66+
})
67+
68+
assert('classicDark theme injects border colors on facet children', {
69+
chart = g2(iris, Sepal.Length ~ Sepal.Width | Species) |> theme_classic_dark()
70+
config = build_config(chart)
71+
(config$children[[1]]$viewStyle$mainStroke %==% 'rgba(255,255,255,0.25)')
72+
(config$children[[1]]$viewStyle$viewStroke %==% 'rgba(255,255,255,0.25)')
73+
})
74+
75+
assert('light theme does not inject border colors on facet children', {
76+
chart = g2(iris, Sepal.Length ~ Sepal.Width | Species)
77+
config = build_config(chart)
78+
(is.null(config$children[[1]]$viewStyle$mainStroke))
79+
(is.null(config$children[[1]]$viewStyle$viewStroke))
80+
})
81+
82+
assert('user-provided mainStroke in viewStyle is not overwritten by dark theme', {
83+
chart = g2(mtcars, hp ~ mpg | cyl) |> theme_dark() |>
84+
mark_point(viewStyle = list(mainStroke = '#ff0000'))
85+
config = build_config(chart)
86+
(config$children[[1]]$viewStyle$mainStroke %==% '#ff0000')
87+
})
88+

0 commit comments

Comments
 (0)