change paratext extension to work as an iframe in paratext#1750
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis update modifies both backend and frontend components. The backend adjusts server rendering security policies and ensures correct working directory setup. The frontend transitions from a custom web component to an iframe-based viewer, updates build tasks and dependencies, enhances CSS for iframe integration, and adds cross-window messaging to display notifications in the viewer. Changes
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
frontend/platform.bible-extension/src/main.ts (1)
29-31: Consider more specific port configuration for better security.While restricting to localhost is good for security, the wildcard port (
*) could allow connections to unintended services running on localhost.Consider using a specific port or range:
-allowedFrameSources: ['http://localhost:*'] +allowedFrameSources: ['http://localhost:29348']Or if multiple ports are needed, specify them explicitly:
-allowedFrameSources: ['http://localhost:*'] +allowedFrameSources: ['http://localhost:29348', 'http://localhost:29349']
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/platform.bible-extension/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
backend/FwLite/FwLiteWeb/FwLiteWebServer.cs(1 hunks)backend/FwLite/FwLiteWeb/Program.cs(1 hunks)frontend/platform.bible-extension/Taskfile.yml(1 hunks)frontend/platform.bible-extension/package.json(1 hunks)frontend/platform.bible-extension/src/extension-template.web-view.tsx(2 hunks)frontend/platform.bible-extension/src/main.ts(2 hunks)frontend/platform.bible-extension/src/styles.css(1 hunks)frontend/viewer/src/ShadcnProjectView.svelte(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/platform.bible-extension/src/extension-template.web-view.tsx (1)
frontend/platform.bible-extension/src/types/fw-lite-extension.d.ts (1)
FindEntryEvent(8-11)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Build UI / publish-ui
- GitHub Check: Build API / publish-api
- GitHub Check: Build FW Lite and run tests
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: check-and-lint
- GitHub Check: frontend
- GitHub Check: Analyze (csharp)
🔇 Additional comments (13)
frontend/platform.bible-extension/src/styles.css (1)
10-12: LGTM! Border removal is appropriate for seamless iframe integration.The border removal ensures the iframe integrates seamlessly without visual boundaries.
frontend/platform.bible-extension/package.json (1)
77-79: LGTM! Dependency removal aligns with iframe-based architecture.Removing the local "viewer" dependency is appropriate since the viewer is now loaded via iframe rather than imported as a component.
frontend/platform.bible-extension/src/main.ts (1)
118-118: LGTM! Code formatting improvement.The blank line improves readability before the process spawn call.
backend/FwLite/FwLiteWeb/FwLiteWebServer.cs (1)
123-126: LGTM: Correctly implements CSP for iframe embedding.The Content Security Policy configuration appropriately allows framing from the same origin and localhost with any port, which aligns with the iframe-based architecture implemented in the frontend components.
frontend/platform.bible-extension/src/extension-template.web-view.tsx (3)
4-4: LGTM: Correctly added useRef import.The useRef import is properly added to support the iframe reference functionality.
10-10: LGTM: Proper iframe ref initialization.The iframe ref is correctly typed and initialized for managing the iframe element.
32-32: LGTM: Properly implemented iframe element.The iframe element correctly uses the ref and src attributes for the iframe-based architecture.
frontend/viewer/src/ShadcnProjectView.svelte (2)
24-24: LGTM: Proper import for notifications.The AppNotification import is correctly added to support displaying cross-frame messages.
60-60: LGTM: Proper window event listener binding.The window message event listener is correctly bound using Svelte's event binding syntax.
frontend/platform.bible-extension/Taskfile.yml (4)
15-15: LGTM: Correctly removed viewer component dependency.The removal of
build-viewer-componentdependency aligns with the architectural shift from embedded components to iframe-based integration.
20-20: LGTM: Consistent dependency update for run task.The run task correctly mirrors the package task dependency changes.
25-25: LGTM: Proper dependency chain for iframe architecture.Adding
build-vieweras a dependency tobuild-fw-lite-webensures the viewer application is built before the web server, which is correct for the iframe-based approach.
28-30: LGTM: Task rename and command update align with architecture change.The task rename from
build-viewer-componenttobuild-viewerand the command change tobuild-appcorrectly reflect the shift from building a component library to building a standalone application for iframe embedding.
myieye
left a comment
myieye
left a comment
There was a problem hiding this comment.
Nice work getting this working again! 🎉
And tanks or the comments for our future selves 😉
I did not try running this, because that would take me a while and I figure we'll run it often enough as soon as we start getting serious about it.
Co-authored-by: Tim Haasdyk <tim_haasdyk@sil.org>
hahn-kev
added
the
💻 FW Lite
2 participants
depends on paranext/paranext-core#1651