Conversation
✅ Deploy Preview for project-idea-board ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| - type: protocolLink | ||
| name: Culture And Freezing Methods | ||
| url: https://cell-catalog.allencell.org/pdf/AICS_WTC_cell_culture_protocol.pdf | ||
| - type: protocolFile | ||
| name: Cell culture protocol | ||
| file: /img/cell_culture_protocol.pdf |
There was a problem hiding this comment.
The type field is created by Decap when you use a variable type widget.
There was a problem hiding this comment.
Pull request overview
Adds support for protocols being either a local uploaded PDF or an external URL, wiring the Decap CMS schema to Gatsby’s GraphQL types and updating frontend rendering accordingly.
Changes:
- Update Decap CMS config to allow variable-type protocol entries (link vs file).
- Update Gatsby schema + page query to support
Protocol { type, name, url, file }. - Update Materials & Methods UI to render protocol links from either
urlorfile.publicURL.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| static/admin/config.yml | Adds variable-type list configuration for protocol link/file entries |
| src/templates/idea-post.tsx | Updates GraphQL query for new protocol shape (and adds debug logging) |
| src/pages/ideas/dev-example.md | Updates example frontmatter to new protocol structure |
| src/components/MaterialsAndMethods.tsx | Renders protocol anchor href based on link vs file type |
| gatsby-node.js | Replaces ProtocolItem with Protocol schema including optional url/file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| title, | ||
| materialsAndMethods, | ||
| }) => { | ||
|
|
There was a problem hiding this comment.
Remove the debug console.log before merge/release, or gate it behind a development-only check. Leaving this in will spam browser consoles and can leak content in production logs.
| if (process.env.NODE_ENV !== "production") { | |
| console.log("protocols:", materialsAndMethods.protocols); | |
| } |
| {protocols.map((item, index) => { | ||
| const href = | ||
| item.type === "protocolLink" | ||
| ? item.url | ||
| : item.file?.publicURL; | ||
| return ( | ||
| <li key={index}> | ||
| <a | ||
| href={href ?? "#"} | ||
| target="_blank" | ||
| rel="noreferrer" | ||
| > | ||
| {item.name} | ||
| </a> | ||
| </li> | ||
| ); | ||
| })} |
There was a problem hiding this comment.
Opening links in a new tab should include rel=\"noopener\" to prevent reverse-tabnabbing (keep noreferrer if desired). Also, falling back to href=\"#\" creates a misleading clickable link; consider conditionally rendering a non-link element (or disabling the anchor) when href is missing instead of using #.
| type MaterialsAndMethods { | ||
| dataset: MarkdownRemark @link(by: "frontmatter.name") | ||
| protocols: [ProtocolItem!]! | ||
| protocols: [Protocol!]! |
There was a problem hiding this comment.
In the CMS config, protocols is marked required: false, but the schema declares protocols: [Protocol!]! (non-null). If a markdown entry omits materialsAndMethods.protocols, Gatsby schema/query resolution can fail at build time. Consider making this field nullable (e.g., [Protocol!]) or making the CMS field required so the data always exists.
| protocols: [Protocol!]! | |
| protocols: [Protocol!] |
There was a problem hiding this comment.
We have a resolver to handle this case and provide an empty array in that case.
| url: https://cell-catalog.allencell.org/pdf/AICS_WTC_cell_culture_protocol.pdf | ||
| - type: protocolFile | ||
| name: Cell culture protocol | ||
| file: /img/cell_culture_protocol.pdf |
There was a problem hiding this comment.
The schema uses file: File @fileByRelativePath, which typically expects a relative path like img/cell_culture_protocol.pdf (no leading slash). Using /img/... may prevent Gatsby from resolving the File node, resulting in file being null and the UI falling back to #. Align the stored value with what @fileByRelativePath expects (or adjust the CMS public_folder/value format accordingly).
| file: /img/cell_culture_protocol.pdf | |
| file: img/cell_culture_protocol.pdf |
There was a problem hiding this comment.
Not sure about this, it's working as is, and its the same way the img file path for the banner was written.
| type: String! | ||
| name: String! | ||
| url: String | ||
| file: File @fileByRelativePath |
There was a problem hiding this comment.
This and the query are where I think we could get a little more into the weeds with graphql especially if we had more complex varied types, but this is fine for now IMHO.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…-board into feature/protocols
1 participant
Problem
Advances #7 and #27
Solution
This is to set up the protocols data structure in materials and methods, and to explore on configuration option in decap/gatsby architecture.
I'm assuming for now that protocols are PDFs, and we might have situations where we want a local file upload or an url.
We could take in strings and handle the question of discerning file vs url in the front end, but this approach handles it a bit more cleanly I think, and I like the way it prevents a small bit of clutter in the admin UI.
On the decap side we use the
typesfield to make a variable type widget:https://decapcms.org/docs/variable-type-widgets/
When a user goes to add a protocol:
They get an option:
On the gatsby side I'm just using a single type with optional fields:
I think we should be able, in theory, to do this in a stricter way with interface and union types in
graphqlbut I think this is good enough for now.https://graphql.org/learn/schema/#interface-types
Admin site users won't be able to enter conflicting fields, a dev could, but it would silently just prefer one over the other on the front end.