Adds embed route for RelatedAggregateGraph#516
Conversation
WalkthroughIntroduces an embed-specific route and layout for related aggregate graphs. Adds a page that composes a DOI from route params and renders RelatedAggregateGraph within Suspense, passing isEmbed=true. Updates RelatedAggregateGraph to accept an optional isEmbed prop that adjusts column offset behavior. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as Browser
participant R as Next.js Router
participant L as EmbedLayout
participant P as EmbedRelatedAggregateGraphPage
participant S as Suspense
participant G as RelatedAggregateGraph
U->>R: Request /embed/related-aggregate/{...doi}
R->>L: Render layout
L->>P: Render page with params.doi
P->>P: Join params.doi with "/"
P->>S: Wrap graph in Suspense
S->>G: Render with { doi, isEmbed: true }
note right of G: Uses isEmbed to set layout offset = 0
G-->>U: Graph content (when ready)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/app/(embed)/embed/related-aggregate/[...doi]/page.tsx (1)
12-16: Consider adding error boundary or validation.The component doesn't handle edge cases:
- Missing or empty
params.doi- Invalid DOI format
- Errors during graph rendering
While the
Suspenseboundary handles loading states, consider wrapping the component with error handling, especially for an embeddable widget that external sites will use.Example error handling:
export default async function EmbedRelatedAggregateGraphPage({ params }: Props) { const resolvedParams = await params; if (!resolvedParams.doi || resolvedParams.doi.length === 0) { return <div style={{ padding: '20px' }}>Invalid DOI</div>; } const doi = decodeURIComponent(resolvedParams.doi.join('/')); return ( <div style={{ width: '100vw', height: '100vh', margin: 0, padding: 0 }}> <Suspense fallback={<div>Loading...</div>}> <RelatedAggregateGraph doi={doi} isEmbed={true} /> </Suspense> </div> ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/(embed)/embed/related-aggregate/[...doi]/page.tsx(1 hunks)src/app/(embed)/embed/related-aggregate/layout.tsx(1 hunks)src/app/doi.org/[...doi]/RelatedAggregateGraph.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/app/doi.org/[...doi]/RelatedAggregateGraph.tsx (1)
src/app/doi.org/[...doi]/RelatedContent.tsx (2)
Props(19-21)RelatedContent(23-119)
src/app/(embed)/embed/related-aggregate/layout.tsx (1)
src/app/layout.tsx (1)
RootLayout(44-71)
src/app/(embed)/embed/related-aggregate/[...doi]/page.tsx (2)
src/app/doi.org/[...doi]/RelatedAggregateGraph.tsx (1)
RelatedAggregateGraph(38-79)src/app/doi.org/[...doi]/page.tsx (2)
Page(73-103)Props(18-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: test / cypress (4)
- GitHub Check: test / cypress (3)
- GitHub Check: test / cypress (5)
- GitHub Check: test / cypress (1)
- GitHub Check: test / cypress (2)
🔇 Additional comments (1)
src/app/doi.org/[...doi]/RelatedAggregateGraph.tsx (1)
13-13: LGTM! Clean implementation of embed support.The addition of the
isEmbedprop with a default value offalsemaintains backward compatibility while enabling the conditional layout adjustment for embedded contexts. The pattern mirrors the existingisBotprop approach consistently.Also applies to: 39-39, 69-69
| @@ -0,0 +1,18 @@ | |||
| import React, { Suspense } from 'react'; | |||
| import RelatedAggregateGraph from 'src/app//doi.org/[...doi]/RelatedAggregateGraph'; | |||
There was a problem hiding this comment.
Fix import path typo.
The import path contains a double slash: src/app//doi.org/[...doi]/RelatedAggregateGraph (note the // after app).
Apply this diff:
-import RelatedAggregateGraph from 'src/app//doi.org/[...doi]/RelatedAggregateGraph';
+import RelatedAggregateGraph from 'src/app/doi.org/[...doi]/RelatedAggregateGraph';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import RelatedAggregateGraph from 'src/app//doi.org/[...doi]/RelatedAggregateGraph'; | |
| import RelatedAggregateGraph from 'src/app/doi.org/[...doi]/RelatedAggregateGraph'; |
🤖 Prompt for AI Agents
In src/app/(embed)/embed/related-aggregate/[...doi]/page.tsx around line 2, the
import path has a double slash
("src/app//doi.org/[...doi]/RelatedAggregateGraph"); fix it by correcting the
path to remove the extra slash so it imports from
"src/app/doi.org/[...doi]/RelatedAggregateGraph" (ensure the adjusted relative
path matches project structure and run a quick compile to verify).
| type Props = { | ||
| params: { doi: string[] } | ||
| }; | ||
|
|
||
| export default function EmbedRelatedAggregateGraphPage({ params }: Props) { | ||
| const doi = params.doi.join('/'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify params handling and add URL decoding.
Two concerns:
-
Params type inconsistency: The main DOI page at
src/app/doi.org/[...doi]/page.tsx(lines 17-21, 72-74) treatsparamsasPromise<{ doi: string[] }>and awaits it, but this component treats it as synchronous. This could cause runtime errors depending on Next.js routing behavior. -
Missing URL decoding: The main page uses
decodeURIComponent(params.doi.join('/'))(line 73), but this embed page omits decoding. DOIs with special characters (e.g.,10.1000/test%2Fpath) will break.
Run the following script to verify how params are handled in other dynamic routes:
Apply this diff if params should be async (recommended based on main page pattern):
-type Props = {
- params: { doi: string[] }
+type Props = {
+ params: Promise<{ doi: string[] }>
};
-export default function EmbedRelatedAggregateGraphPage({ params }: Props) {
- const doi = params.doi.join('/');
+export default async function EmbedRelatedAggregateGraphPage({ params }: Props) {
+ const resolvedParams = await params;
+ const doi = decodeURIComponent(resolvedParams.doi.join('/'));🏁 Script executed:
#!/bin/bash
# Check params handling patterns across dynamic routes
rg -n -A 3 'params.*Promise' --type=tsx --type=ts
rg -n -A 3 'decodeURIComponent.*params' --type=tsx --type=tsLength of output: 55
🏁 Script executed:
#!/bin/bash
# Locate all page.tsx files under any `[...doi]` directory
echo "Found dynamic DOI page files:"
fd -t f -e tsx '\[\.\.\.doi\]' src/app
# For each file, inspect params handling and URL decoding
for f in $(fd -t f -e tsx '\[\.\.\.doi\]' src/app); do
echo
echo "----- $f -----"
rg -n -C2 'export default (async )?function' --glob "$f"
rg -n 'await params' --glob "$f"
rg -n 'decodeURIComponent' --glob "$f"
doneLength of output: 29
🏁 Script executed:
#!/bin/bash
echo "Listing all .tsx files under src/app:"
fd -e tsx src/app
echo
echo "Filtering for '[...doi]' in paths:"
find src/app -type f -name '*.tsx' | grep '\[...doi\]'
echo
for f in $(find src/app -type f -name '*.tsx' | grep '\[...doi\]'); do
echo
echo "----- $f -----"
head -n 20 "$f"
echo "- Async export:"
rg -n '^export default.*async' --glob "$f" || echo " none"
echo "- Await params:"
rg -n 'await params' --glob "$f" || echo " none"
echo "- decodeURIComponent:"
rg -n 'decodeURIComponent' --glob "$f" || echo " none"
doneLength of output: 6743
🏁 Script executed:
#!/bin/bash
# Locate embed and main DOI page files
embed=$(find src/app -type f -path "*/(embed)/*/page.tsx")
main=$(find src/app -type f -path "*/doi.org/*/page.tsx" | grep '\[...doi\]/page.tsx')
echo "Embed file: $embed"
echo "Main file: $main"
echo
echo "----- EMBED PAGE ($embed) -----"
sed -n '1,200p' "$embed"
echo
echo "----- MAIN PAGE ($main) -----"
sed -n '1,200p' "$main"Length of output: 3555
Decode DOI in embed page
In src/app/(embed)/embed/related-aggregate/[...doi]/page.tsx replace:
const doi = params.doi.join('/');with
const doi = decodeURIComponent(params.doi.join('/'));🤖 Prompt for AI Agents
In src/app/(embed)/embed/related-aggregate/[...doi]/page.tsx around lines 4 to
9, the DOI is being reconstructed from route params without decoding URL-encoded
characters; replace the current reconstruction with decoding by calling
decodeURIComponent on the joined params (i.e.,
decodeURIComponent(params.doi.join('/'))) so the DOI contains original
characters like slashes or percent-encoded segments.
| import React from 'react'; | ||
|
|
||
| export default function EmbedLayout({ children }: { children: React.ReactNode }) { | ||
| return ( | ||
| <html> | ||
| <body> | ||
| <div> | ||
| {children} | ||
| </div> | ||
| </body> | ||
| </html> | ||
| ); | ||
| } No newline at end of file |
There was a problem hiding this comment.
Add proper HTML document structure for embed layout.
The layout is missing critical HTML elements that affect accessibility, SEO, and proper rendering:
- Missing
langattribute on<html>tag (accessibility requirement for screen readers) - Missing
<head>section with essential meta tags:<meta charset="utf-8" /><meta name="viewport" content="width=device-width, initial-scale=1" />
- No CSS classes or styling to ensure consistent rendering
Apply this diff to add proper HTML structure:
import React from 'react';
export default function EmbedLayout({ children }: { children: React.ReactNode }) {
return (
- <html>
+ <html lang="en">
+ <head>
+ <meta charSet="utf-8" />
+ <meta name="viewport" content="width=device-width, initial-scale=1" />
+ </head>
<body>
- <div>
- {children}
- </div>
+ {children}
</body>
</html>
);
}Note: The extra <div> wrapper is unnecessary—{children} can be rendered directly in <body>.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import React from 'react'; | |
| export default function EmbedLayout({ children }: { children: React.ReactNode }) { | |
| return ( | |
| <html> | |
| <body> | |
| <div> | |
| {children} | |
| </div> | |
| </body> | |
| </html> | |
| ); | |
| } | |
| import React from 'react'; | |
| export default function EmbedLayout({ children }: { children: React.ReactNode }) { | |
| return ( | |
| <html lang="en"> | |
| <head> | |
| <meta charSet="utf-8" /> | |
| <meta name="viewport" content="width=device-width, initial-scale=1" /> | |
| </head> | |
| <body> | |
| {children} | |
| </body> | |
| </html> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/app/(embed)/embed/related-aggregate/layout.tsx around lines 1-13, the
HTML document lacks a lang attribute, a head section with essential meta tags,
and unnecessary wrapper markup; modify the root html tag to include lang="en",
add a <head> containing <meta charSet="utf-8" /> and <meta name="viewport"
content="width=device-width, initial-scale=1" />, remove the extra <div> so
{children} is rendered directly inside <body>, and add a CSS class on <body>
(e.g., className="embed-root") so global or local styles can target this layout
for consistent rendering.
1 participant
Purpose
closes: Add github issue that originated this PR
Approach
Open Questions and Pre-Merge TODOs
Learning
Types of changes
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Reviewer, please remember our guidelines:
Summary by CodeRabbit