feat: add gdocs component for bespoke viz#6050
Conversation
|
Quick links (staging server):
Login:
Archive:SVG tester:Number of differences (graphers): 0 ✅ Edited: 2026-03-10 11:40:54 UTC |
2310235 to
15faf3d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ff8b3c623
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Create a container div inside the shadow root for the component to render into | ||
| const mountContainer = document.createElement("div") | ||
| mountContainer.className = "bespoke-container" | ||
| shadowRoot!.appendChild(mountContainer) |
There was a problem hiding this comment.
Clear shadow DOM on rehydrate to avoid duplicates
Every effect run appends a new mount container (and, earlier, a new <link> for CSS) into the same shadow root, but the cleanup only calls the disposer. When block.config/variant changes or the component remounts, old DOM/CSS can accumulate unless each bespoke bundle explicitly removes it, leading to duplicate visuals or leaked nodes. Consider reusing a single container or clearing the shadow root in cleanup before remounting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
How would I go about doing this?
There was a problem hiding this comment.
Maybe you could still fix this, but it seems like... not a big deal to me, given that block.config and variant won't change, due to how we only ever hydrate the page once.
There was a problem hiding this comment.
This is where the action is
site/bespokeComponentRegistry.ts
Outdated
| test: { | ||
| scriptUrl: | ||
| "https://owid-public.owid.io/marcel-bespoke-data-viz-02-2026/poverty-plots/income-plots.mjs", | ||
| cssUrls: [ | ||
| "https://owid-public.owid.io/marcel-bespoke-data-viz-02-2026/poverty-plots/income-plots.css", | ||
| ], | ||
| }, |
There was a problem hiding this comment.
This is an example registry entry; and what is powering the demo that you're seeing. I'll remove it before we merge the PR.
There was a problem hiding this comment.
Right so we still need to figure out the way we write and generate these bundles. I guess for starters it doesn't need to be too systematized.
db/model/Gdoc/rawToArchie.ts
Outdated
| yield* propertyToArchieMLString("variant", block.value) | ||
| yield* propertyToArchieMLString("size", block.value) | ||
| if (block.value.config) { | ||
| // TODO this doesn't support deep nesting, which in theory the type Record<string, unknown> allows for |
There was a problem hiding this comment.
I wonder if it'd be smart to just decide now that nesting isn't allowed, and check for it in parsing/validation.
i.e. we'd force:
{.config}
aspect-ratio-small: 1
aspect-ratio-large: 2
{}
instead of
{.config}
{.aspect-ratio}
small: 1
large: 2
{}
{}
I don't have a strong opinion on this, but maybe it makes it a little easier to work with and a little harder to write? (Though, the most common archieML breakage we encounter is authors mistakenly missing/closing brackets, so this might make that failure mode a little rarer)
ikesau
left a comment
There was a problem hiding this comment.
All the Archie stuff looks good! Will QA this tomorrow before full approval 👍
ikesau
left a comment
There was a problem hiding this comment.
Really clean proof of concept!
I left a few thoughts on some smaller details, but the overall approach makes sense to me.
| .with({ type: "bespoke-component" }, (block) => ( | ||
| <BespokeComponent | ||
| className={getLayout( | ||
| `bespoke-component--${block.size}`, |
There was a problem hiding this comment.
What if we just rendered the container at 12 columns no matter what, and the viz itself took care of its sizing?
Doing it this way (with block.size) means that the logic for sizing a bespoke data viz will exist in at least 2 places:
- The archie wrapper component (adding a dependency on our site's CSS (unlikely to change, but technically possible)
- The bespoke viz rendering & CSS
Given that there's a config object that we can pass down (if we want a big and small version of the same viz) I feel like we might as well centralize?
There was a problem hiding this comment.
Hm, I'm not sure how this will play along with putting this component into other, small components - whether it's an aside or side-by-side or anything else - but I'll keep it in mind when thinking about grid-aware sizing.
| // Create a container div inside the shadow root for the component to render into | ||
| const mountContainer = document.createElement("div") | ||
| mountContainer.className = "bespoke-container" | ||
| shadowRoot!.appendChild(mountContainer) |
There was a problem hiding this comment.
Maybe you could still fix this, but it seems like... not a big deal to me, given that block.config and variant won't change, due to how we only ever hydrate the page once.
| position: "relative", | ||
| minHeight: 300, |
There was a problem hiding this comment.
So this is where we could implement the aspect ratio system we were talking about.
If we go with the "centralize sizing information in one place" idea that I mentioned in the other comment, does that mean we'll also need a max-width attribute for each variant? 🤔
{.config}
desktop-aspect-ratio: 1.618
max-width: 900px
mobile-aspect-ratio: 1
// we'd probably want mobile max width to always be 100%
{}
Or if we want it to be more consistent, use a size enum.
site/bespokeComponentRegistry.ts
Outdated
| test: { | ||
| scriptUrl: | ||
| "https://owid-public.owid.io/marcel-bespoke-data-viz-02-2026/poverty-plots/income-plots.mjs", | ||
| cssUrls: [ | ||
| "https://owid-public.owid.io/marcel-bespoke-data-viz-02-2026/poverty-plots/income-plots.css", | ||
| ], | ||
| }, |
There was a problem hiding this comment.
Right so we still need to figure out the way we write and generate these bundles. I guess for starters it doesn't need to be too systematized.
|
This PR has had no activity within the last two weeks. It is considered stale and will be closed in 3 days if no further activity is detected. |
c77eeb7 to
95ed30d
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5d185ae4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (error) { | ||
| return <BespokeError className={className} message={error} /> |
There was a problem hiding this comment.
Reset error state before rendering fallback-only branch
After any load failure, setError(...) causes the component to render only BespokeError, which removes the containerRef node. Because the effect exits early on if (!container) return, the same mounted instance cannot hydrate again on later prop updates (e.g. live-preview edits or retry-worthy transient CDN failures), so the block stays permanently broken until a full remount. Clearing error before retry/hydration (and keeping a mount container available) avoids this dead-end state.
Useful? React with 👍 / 👎.
| const mountContainer = document.createElement("div") | ||
| mountContainer.className = "bespoke-container" | ||
| shadowRoot!.appendChild(mountContainer) | ||
|
|
There was a problem hiding this comment.
Remove stale shadow nodes when rehydrating bespoke block
Each effect run appends a new bespoke-container into the shadow root, but cleanup only calls an optional module-provided disposer and never removes previously appended DOM/styles. Since mount is explicitly allowed to return void, updates to bundle, variant, or config on the same component instance can accumulate old mounts and leaked styles, leading to duplicated or stale UI. Explicitly clearing/replacing injected shadow-root nodes during rehydration would prevent this.
Useful? React with 👍 / 👎.
Adds a
{.bespoke-component}component.It has an in-code registry of component "bundles" (which are just pointers to JS & CSS URLs), and the JS entrypoint needs to export a
method.
The component is a client-only component; it currently doesn't do anything before hydration and just renders an (infinite) loading indicator.
What is
variant?The
variantis an optional pointer for multiple visualizations to be part of the same bundle, but to be embedded in different spots inside the gdoc - but still being able to share state.In theory, this could also be achieved by just using
config, but I feel like it's a common-enough use case that we want to have an "approved" way to do this.How this works
Technically, we load the JS file using dynamic await, and then call
mounton a div we create inside of a Shadow DOM.The CSS is also loaded into the Shadow DOM, and thus the bespoke component is fully encapsulated when it comes to CSS.
Drawbacks of Shadow DOM
This creates a bunch of small problems also, though: We need to use
:hostrather than:rootinside the CSS to define global variables, and need to ensure that any code that mounts elements inside portals (e.g. tooltip code likefloating-uiin my case) mounts its element inside the Shadow DOM, because if it gets mounted outside it doesn't have CSS styles.Try it out
You can preview this here (this is the doc)
In the demo, you can easily see how the two charts are connected in state. It's easy to disconnect their state (but the bespoke implementation needs to handle that), but the ability to have them connected is really useful in some of our use cases.
TODOs
Open questions
.scriptcomponent now?Screenshot