Add ROI calculator to Overview section#4
Conversation
Embeds the idOS ROI calculator (github.com/lbardet/idos-roi-calculator) as a full-viewport iframe page, listed in the sidebar under Overview below Market focus. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@lbardet is attempting to deploy a commit to the idOS Engineering Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughA new Docusaurus page ( ChangesROI Calculator Page Addition
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docusaurus-site/src/pages/roi-calculator.js`:
- Around line 8-12: The hardcoded `top: 60` value in the style object is brittle
because it assumes a fixed navbar height and will require multiple updates if
the navbar height ever changes. Extract this `top: 60` offset into a shared
constant (either in a constants file or at the top of the file) or define it as
a CSS variable, then reference that constant or variable in the style object
instead of the hardcoded value. This ensures the navbar offset can be maintained
in a single location and keeps related values synchronized across the codebase.
- Around line 14-18: The iframe element in the roi-calculator.js file that loads
"/roi-calculator.html" is missing security hardening. Add a sandbox attribute
with the value allow-same-origin allow-scripts to the iframe element to restrict
its capabilities and reduce the attack surface. Additionally, if the ROI
calculator requires fullscreen functionality, add the allowFullscreen attribute
to the iframe as well.
- Line 1: Remove the React import statement from the top of the
roi-calculator.js file. The line importing React from 'react' is unnecessary
with React 19's modern JSX transform that Docusaurus 3.10.1 uses by default, and
removing it aligns with current React best practices.
In `@SUMMARY.md`:
- Line 9: The SUMMARY.md file contains a TOC entry referencing a non-existent
`roi-calculator.md` file, while the ROI calculator is actually implemented as a
React component and already registered in sidebars.ts. Remove the line `* [ROI
calculator](roi-calculator.md)` from SUMMARY.md since the entry is redundant
with the sidebar navigation and SUMMARY.md should only contain markdown-based
documentation entries, not React page routes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4c904c34-afd2-46be-8025-0065e0e78cba
📒 Files selected for processing (4)
SUMMARY.mddocusaurus-site/sidebars.tsdocusaurus-site/src/pages/roi-calculator.jsdocusaurus-site/static/roi-calculator.html
| @@ -0,0 +1,22 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Remove unnecessary React import for React 19 compatibility.
With React 19's modern JSX transform enabled (which Docusaurus 3.10.1 uses by default), the React import on Line 1 is no longer required. Removing it aligns with current React 19 best practices.
♻️ Proposed fix
-import React from 'react';
import Layout from '`@theme/Layout`';📝 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'; | |
| import Layout from '`@theme/Layout`'; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docusaurus-site/src/pages/roi-calculator.js` at line 1, Remove the React
import statement from the top of the roi-calculator.js file. The line importing
React from 'react' is unnecessary with React 19's modern JSX transform that
Docusaurus 3.10.1 uses by default, and removing it aligns with current React
best practices.
| position: 'fixed', | ||
| top: 60, | ||
| left: 0, | ||
| right: 0, | ||
| bottom: 0, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff
Extract hardcoded navbar offset into a constant or CSS variable.
The top: 60 offset assumes a fixed navbar height, which creates fragility if the navbar height changes in the future. Consider extracting this to a shared constant or CSS variable that can be maintained centrally.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docusaurus-site/src/pages/roi-calculator.js` around lines 8 - 12, The
hardcoded `top: 60` value in the style object is brittle because it assumes a
fixed navbar height and will require multiple updates if the navbar height ever
changes. Extract this `top: 60` offset into a shared constant (either in a
constants file or at the top of the file) or define it as a CSS variable, then
reference that constant or variable in the style object instead of the hardcoded
value. This ensures the navbar offset can be maintained in a single location and
keeps related values synchronized across the codebase.
| <iframe | ||
| src="/roi-calculator.html" | ||
| style={{ width: '100%', height: '100%', border: 'none', display: 'block' }} | ||
| title="idOS ROI Calculator" | ||
| /> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add sandbox attribute to iframe for security; consider allowFullscreen if needed.
The iframe lacks a sandbox attribute, which is a security best practice even for internal content. This limits the attack surface if the embedded content is compromised. Additionally, if the ROI calculator UI requires fullscreen capability, the allowFullscreen attribute should be present.
🛡️ Proposed fix (with sandbox + allowFullscreen for full interactivity)
<iframe
src="/roi-calculator.html"
+ sandbox="allow-same-origin allow-scripts allow-popups allow-forms"
+ allowFullscreen
style={{ width: '100%', height: '100%', border: 'none', display: 'block' }}
title="idOS ROI Calculator"
/>If the calculator only needs to display static content without forms or popups, use:
sandbox="allow-same-origin allow-scripts"
📝 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.
| <iframe | |
| src="/roi-calculator.html" | |
| style={{ width: '100%', height: '100%', border: 'none', display: 'block' }} | |
| title="idOS ROI Calculator" | |
| /> | |
| <iframe | |
| src="/roi-calculator.html" | |
| sandbox="allow-same-origin allow-scripts allow-popups allow-forms" | |
| allowFullscreen | |
| style={{ width: '100%', height: '100%', border: 'none', display: 'block' }} | |
| title="idOS ROI Calculator" | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docusaurus-site/src/pages/roi-calculator.js` around lines 14 - 18, The iframe
element in the roi-calculator.js file that loads "/roi-calculator.html" is
missing security hardening. Add a sandbox attribute with the value
allow-same-origin allow-scripts to the iframe element to restrict its
capabilities and reduce the attack surface. Additionally, if the ROI calculator
requires fullscreen functionality, add the allowFullscreen attribute to the
iframe as well.
| * [Why idOS?](why-idos.md) | ||
| * [What is self-custodial data?](what-is-self-custodial-data.md) | ||
| * [Market focus](market-focus.md) | ||
| * [ROI calculator](roi-calculator.md) |
There was a problem hiding this comment.
Fix inconsistency: TOC entry references non-existent roi-calculator.md.
The SUMMARY.md entry links to roi-calculator.md, but the ROI calculator is implemented as a React page component (roi-calculator.js), not a markdown file. This creates a cross-file inconsistency with sidebars.ts, which correctly links to the /roi-calculator route.
SUMMARY.md is designed for markdown-based documentation, not React pages. Either:
- Remove the entry from SUMMARY.md (React pages don't belong in TOC), or
- Change the link to
/roi-calculatorto match the sidebar
Option 1 is recommended: since the ROI calculator is already registered in sidebars.ts and appears in the sidebar navigation, the TOC entry is redundant.
✏️ Proposed fix: Remove from TOC
* [What is idOS?](README.md)
* [Why idOS?](why-idos.md)
* [What is self-custodial data?](what-is-self-custodial-data.md)
* [Market focus](market-focus.md)
-* [ROI calculator](roi-calculator.md)
* [Example use cases](example-use-cases/README.md)Alternative fix: Link to route instead of markdown file
-* [ROI calculator](roi-calculator.md)
+* [ROI calculator](/roi-calculator)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SUMMARY.md` at line 9, The SUMMARY.md file contains a TOC entry referencing a
non-existent `roi-calculator.md` file, while the ROI calculator is actually
implemented as a React component and already registered in sidebars.ts. Remove
the line `* [ROI calculator](roi-calculator.md)` from SUMMARY.md since the entry
is redundant with the sidebar navigation and SUMMARY.md should only contain
markdown-based documentation entries, not React page routes.
1 participant
Summary
src/pages/roi-calculator.js) with the calculator HTML embedded via iframeTest plan
/roi-calculatorand confirm the calculator loads full-screen🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation