Conversation
✅ Deploy Preview for cell-catalog ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Adds Cloudinary-backed image uploads to the Decap CMS admin flow while preserving existing local-image rendering by introducing a dual image model (Gatsby image data vs external URL) and plumbing that URL through Gatsby GraphQL.
Changes:
- Added a custom Decap CMS widget (
cloudinary-image) that uploads to Cloudinary and writes the resulting URL into frontmatter. - Introduced
ImageRenderer+ImageSourceto render eitherGatsbyImagedata or external<img src=...>URLs, and updated consumers accordingly. - Added an
image_urlGraphQL field (viacreateResolvers) and updated relevant queries to fetch it alongside existing image fields.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| static/admin/config.yml | Switches image widgets to the new cloudinary-image widget (including header background). |
| src/utils/mediaUtils.ts | Adds isExternalUrl, updates image unpacking to support image_url, and changes thumbnail extraction to return ImageSource. |
| src/templates/disease-cell-line.tsx | Queries image_url alongside existing image fields. |
| src/templates/cell-line.tsx | Queries image_url alongside existing image fields (including media + diagrams + rnaseq). |
| src/style/images-and-videos.module.css | Adjusts media container height styling. |
| src/components/shared/ImageRenderer.tsx | New shared renderer for Gatsby images vs external URLs. |
| src/components/shared/DiagramCard.tsx | Uses ImageRenderer and updates prop typing to accept ImageSource. |
| src/components/Thumbnail.tsx | Uses ImageRenderer and updates prop typing to accept ImageSource. |
| src/components/SubPage/convert-data.ts | Uses unpackImageData to normalize images into ImageSource. |
| src/components/ParentalLineModal.tsx | Updates thumbnail rendering to support ImageSource via ImageRenderer. |
| src/components/ImagesAndVideo/MediaCard.tsx | Replaces direct GatsbyImage usage with ImageRenderer. |
| src/components/ImagesAndVideo/ImagePreviewGroup.tsx | Supports previewing external URLs by bypassing getSrc when needed. |
| src/components/CellLineTable/SharedColumns.tsx | Updates thumbnail rendering logic to support external URLs. |
| src/component-queries/types.ts | Introduces ImageSource and updates frontmatter/result types for Cloudinary URL support. |
| src/component-queries/NormalCellLines.tsx | Adds image_url to the table query. |
| src/component-queries/DiseaseCellLines.tsx | Adds image_url to the disease table query. |
| src/cms/widgets/CloudinaryImageWidget.tsx | New Cloudinary upload widget implementation (script loading, folder derivation, preview). |
| src/cms/cms.tsx | Registers the custom cloudinary-image widget with Decap CMS. |
| src/cms/cloudinaryConfig.ts | Adds Cloudinary widget configuration constants. |
| gatsby-node.js | Adds schema/types/resolvers for image_url and introduces an ImagesAndVideos type. |
Comments suppressed due to low confidence (1)
gatsby-node.js:68
ImagesAndVideosis now explicitly typed but only includesimages. Existing GraphQL queries insrc/templates/cell-line.tsxrequestimages_and_videos { videos { ... } }, so this schema will either drop thevideosfield or rely on inference (which is brittle) and can break builds. Define aVideoWithCaptiontype and addvideos: [VideoWithCaption]toImagesAndVideos(or otherwise include the videos field explicitly) so the schema matches frontmatter usage.
` type ImgWithCaption {
image: File @fileByRelativePath
caption: String
}
type ImagesAndVideos {
images: [ImgWithCaption]
}
type Diagram {
title: String
images: [ImgWithCaption]
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const [localSrc, setLocalSrc] = useState<string | null>(null); | ||
| useEffect(() => { | ||
| if (!hasImage || isUrl) { | ||
| setLocalSrc(null); | ||
| return; | ||
| } | ||
| // getAsset returns an AssetProxy; its path may resolve async | ||
| const asset = getAsset(value, field); | ||
| const src = asset.toString(); | ||
| if (src && !src.endsWith("undefined")) { | ||
| setLocalSrc(src); | ||
| } | ||
| // Poll briefly for the blob to be populated by the CMS proxy | ||
| const interval = setInterval(() => { | ||
| const resolved = getAsset(value, field).toString(); | ||
| if (resolved && resolved !== src) { | ||
| setLocalSrc(resolved); | ||
| clearInterval(interval); | ||
| } | ||
| }, 500); | ||
| return () => clearInterval(interval); |
There was a problem hiding this comment.
The preview polling interval can run indefinitely if getAsset(value, field).toString() never changes (e.g., it already returns a stable value, or it returns an unresolved placeholder forever). Add a max attempt/time limit (and/or stop polling once a non-empty, non-"undefined" src is observed) to avoid leaking intervals while the editor stays open.
| export const CLOUDINARY_CLOUD_NAME = "dkg6lnogl"; | ||
| export const CLOUDINARY_UPLOAD_PRESET = "allen-cell"; | ||
| export const CLOUDINARY_API_KEY = "989839737788897"; |
There was a problem hiding this comment.
Cloudinary identifiers are hardcoded in-repo. Even though the API key isn’t a secret, hardcoding the cloud name/preset/key makes it difficult to deploy forks or alternate environments (preview vs prod) safely. Prefer reading these from environment variables (e.g., process.env.GATSBY_CLOUDINARY_*) with a clear fallback/error so configuration is deploy-time, not code-time.
| export const CLOUDINARY_CLOUD_NAME = "dkg6lnogl"; | |
| export const CLOUDINARY_UPLOAD_PRESET = "allen-cell"; | |
| export const CLOUDINARY_API_KEY = "989839737788897"; | |
| const getRequiredEnvVar = (name: string): string => { | |
| const value = process.env[name]; | |
| if (!value || !value.trim()) { | |
| throw new Error( | |
| `Missing required Cloudinary configuration: ${name}. ` + | |
| "Set this environment variable at deploy time." | |
| ); | |
| } | |
| return value; | |
| }; | |
| export const CLOUDINARY_CLOUD_NAME = getRequiredEnvVar( | |
| "GATSBY_CLOUDINARY_CLOUD_NAME" | |
| ); | |
| export const CLOUDINARY_UPLOAD_PRESET = getRequiredEnvVar( | |
| "GATSBY_CLOUDINARY_UPLOAD_PRESET" | |
| ); | |
| export const CLOUDINARY_API_KEY = getRequiredEnvVar( | |
| "GATSBY_CLOUDINARY_API_KEY" | |
| ); |
| label: "Background Image", | ||
| name: "background", | ||
| widget: "image", | ||
| widget: "cloudinary-image", |
There was a problem hiding this comment.
The header background field is switched to cloudinary-image, which will store a URL string in frontmatter. Pages like src/templates/disease-catalog.tsx currently treat header.background as a FileNode and even use a non-null assertion (imageFile!) before calling getImageSrcFromFileNode, so a Cloudinary URL (which resolves background to null via @fileByRelativePath) can cause a runtime crash. Either keep this field on the standard image widget, or add parallel URL support (e.g., a background_url resolver + template changes to handle both).
| widget: "cloudinary-image", | |
| widget: "image", |
| function loadCloudinaryScript(): Promise<void> { | ||
| if (scriptLoaded) return Promise.resolve(); | ||
| return new Promise((resolve, reject) => { | ||
| const script = document.createElement("script"); | ||
| script.src = | ||
| "https://upload-widget.cloudinary.com/latest/global/all.js"; | ||
| script.onload = () => { | ||
| scriptLoaded = true; | ||
| resolve(); | ||
| }; | ||
| script.onerror = reject; | ||
| document.head.appendChild(script); | ||
| }); |
There was a problem hiding this comment.
loadCloudinaryScript() only gates on a boolean (scriptLoaded). If openUploader is triggered multiple times before the script finishes loading, this will append multiple <script> tags and race the global initialization. Cache the in-flight Promise (e.g., let scriptPromise: Promise<void> | null) and return it while loading to guarantee a single script insertion.
| function loadCloudinaryScript(): Promise<void> { | |
| if (scriptLoaded) return Promise.resolve(); | |
| return new Promise((resolve, reject) => { | |
| const script = document.createElement("script"); | |
| script.src = | |
| "https://upload-widget.cloudinary.com/latest/global/all.js"; | |
| script.onload = () => { | |
| scriptLoaded = true; | |
| resolve(); | |
| }; | |
| script.onerror = reject; | |
| document.head.appendChild(script); | |
| }); | |
| let scriptPromise: Promise<void> | null = null; | |
| function loadCloudinaryScript(): Promise<void> { | |
| if (scriptLoaded) return Promise.resolve(); | |
| if (scriptPromise) return scriptPromise; | |
| scriptPromise = new Promise((resolve, reject) => { | |
| const script = document.createElement("script"); | |
| script.src = | |
| "https://upload-widget.cloudinary.com/latest/global/all.js"; | |
| script.onload = () => { | |
| scriptLoaded = true; | |
| scriptPromise = null; | |
| resolve(); | |
| }; | |
| script.onerror = () => { | |
| scriptPromise = null; | |
| reject(new Error("Failed to load Cloudinary Upload Widget script")); | |
| }; | |
| document.head.appendChild(script); | |
| }); | |
| return scriptPromise; |
| const openUploader = useCallback(async () => { | ||
| await loadCloudinaryScript(); | ||
|
|
||
| const folder = getFolderFromEntry(entry); | ||
| const cloudinary = (window as CloudinaryWindow).cloudinary; | ||
|
|
||
| if (!cloudinary) { | ||
| console.error("Cloudinary upload widget not loaded"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
openUploader awaits loadCloudinaryScript() but doesn’t handle rejections. If the script fails to load (network/CSP), this async handler will throw and can surface as an unhandled promise rejection in the CMS UI. Wrap the await in a try/catch and show a user-visible error state (or at least log and return).
Problem
What is the problem this work solves, including
step 1 of #88
Solution
What I/we did to solve this problem
ImageRendererto handle both gastby images(GatsbyImage) and external urls (<img>)image_urlfield viacreateResolversin gatsby-node.js.imageresolves toGatsbyImage,image_urlis nullimageis null andimage_urlcarries the urlNotes:
This is mainly to show how Cloudinary can work with our current image management. With this change, images can now be uploaded to Cloudinary, while local images rendering still works the same as it does on
main.Things we can decide separately later:
Not yet implemented:
Type of change
Please delete options that are not relevant.
Steps to Verify:
Screenshots (optional):
Show-n-tell images/animations here
Replace Imagefor a local image, the replacement image is uploaded to Cloudinary instead.