Added Dynamic Google Form Integration feature #296
Conversation
WalkthroughAdds a new Google Doc/Form viewer page and route. Introduces navigation logic from the question type selection to either the new Google Form page or an input page. Implements URL parsing and iframe embedding for Google Docs/Forms, with loading and error handling. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Question_Type as Question_Type Page
participant Router
participant Google_doc as Google_doc Page
User->>Question_Type: Select option
alt Option = "dynamic_form"
Question_Type->>Router: navigate("/google-form")
Router-->>Google_doc: Render component
else Other option
Question_Type->>Router: navigate("/input")
end
sequenceDiagram
actor User
participant Google_doc as GoogleDocViewer
participant Parser as URL Parser
participant IFrame as <iframe>
User->>Google_doc: Submit Google URL
Google_doc->>Parser: Validate & classify (Doc vs Form)
alt Valid Google Doc/Form URL
Parser-->>Google_doc: Embed URL
Google_doc->>IFrame: Set src=embed URL (show loading)
IFrame-->>Google_doc: load or error
alt load
Google_doc-->>User: Display content (hide loading)
else error
Google_doc-->>User: Show error (hide loading)
end
else Invalid URL
Parser-->>Google_doc: Error
Google_doc-->>User: Show validation error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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
🧹 Nitpick comments (8)
eduaid_web/src/App.js (2)
9-9: Align component naming with conventions (optional)Prefer PascalCase aliasing for React components for consistency.
Apply this diff and update the usage accordingly:
-import Google_doc from "./pages/Google_doc"; +import GoogleDocViewer from "./pages/Google_doc";Then update the route usage:
<Route path="/google-form" element={<GoogleDocViewer />} />
11-23: Consider lazy‑loading the Google viewer to cut initial bundle sizeLoad this rarely used page on demand.
Minimal example:
-import Google_doc from "./pages/Google_doc"; +import React, { Suspense, lazy } from "react"; +const GoogleDocViewer = lazy(() => import("./pages/Google_doc"));Wrap the route element:
<Route path="/google-form" element={ <Suspense fallback={<div />}> <GoogleDocViewer /> </Suspense> } />eduaid_web/src/pages/Google_doc.jsx (4)
18-31: Harden URL validation and support more Google Forms patternsCurrent detection relies on substring checks and a narrow regex. Prefer validating host and matching pathname for both “/forms/d/e/ID/...” and “/forms/d/ID/...”.
Apply this diff:
- try { - const trimmedUrl = url.trim(); - - if (trimmedUrl.includes("/forms/")) { - const match = trimmedUrl.match(/\/d\/e\/([a-zA-Z0-9-_]+)\//); + try { + const trimmedUrl = url.trim(); + let u; + try { + u = new URL(trimmedUrl); + } catch { + setError("Invalid URL"); + setLoading(false); + return; + } + if (u.hostname !== "docs.google.com") { + setError("URL must be from docs.google.com"); + setLoading(false); + return; + } + + if (/^\/forms\/d\//.test(u.pathname)) { + const match = u.pathname.match(/^\/forms\/d\/(?:e\/)?([a-zA-Z0-9_-]+)/); if (!match) { setError("Invalid Google Form URL"); setLoading(false); return; } - const formId = match[1]; - const formUrl = `https://docs.google.com/forms/d/e/${formId}/viewform?embedded=true`; + const formId = match[1]; + const formUrl = `https://docs.google.com/forms/d/e/${formId}/viewform?embedded=true`; setIframeUrl(formUrl); - } else if (trimmedUrl.includes("/document/")) { - const match = trimmedUrl.match(/\/d\/([a-zA-Z0-9-_]+)(\/|$)/); + } else if (/^\/document\/d\//.test(u.pathname)) { + const match = u.pathname.match(/^\/document\/d\/([a-zA-Z0-9_-]+)/); if (!match) { setError("Invalid Google Doc URL"); setLoading(false); return; } const docId = match[1]; - const docUrl = `https://docs.google.com/document/d/${docId}/export?format=html`; + const docUrl = `https://docs.google.com/document/d/${docId}/export?format=html`; setIframeUrl(docUrl); - console.log(docUrl) } else { setError("URL must be a Google Doc or Form link"); setLoading(false); }
41-41: Remove stray console.logAvoid debug logs in production.
Apply this diff:
- console.log(docUrl)
39-40: Consider an embed‑friendly Docs URL“export?format=html” can look unstyled or be blocked. When possible, prefer published embed:
Alternative:
const docUrl = `https://docs.google.com/document/d/${docId}/pub?embedded=true`;Note: requires “File -> Share -> Publish to the web” on the document.
Please verify with your target docs whether /pub?embedded=true renders reliably versus /export?format=html.
86-93: Minor a11y: add aria-label and consider autoFocusImproves screen reader support and UX.
Apply this diff:
- <input + <input type="url" placeholder="https://docs.google.com/document/.." value={url} onChange={(e) => setUrl(e.target.value)} className="px-4 py-2 rounded-lg w-full bg-[#1e293b] text-white outline-none" required + aria-label="Google Doc or Google Form URL" + autoFocus />eduaid_web/src/pages/Question_Type.jsx (2)
80-83: Remove debug log and support Space key for keyboard selection (optional)Keeps console clean and improves a11y.
Apply this diff:
- if (e.key === "Enter") { - console.log("Wev") - handleOptionClick(option.id); - } + if (e.key === "Enter" || e.key === " ") { + handleOptionClick(option.id); + }
100-110: Optional UX cleanup: avoid double navigationIf you adopt immediate navigation on option click, consider hiding/disabling the “Fire Up” button to prevent redundant actions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
eduaid_web/src/App.js(1 hunks)eduaid_web/src/pages/Google_doc.jsx(1 hunks)eduaid_web/src/pages/Question_Type.jsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
eduaid_web/src/pages/Question_Type.jsx (2)
eduaid_web/src/pages/Previous.jsx (1)
navigate(10-10)eduaid_web/src/pages/Text_Input.jsx (1)
handleSaveToLocalStorage(49-76)
eduaid_web/src/App.js (1)
eduaid_web/src/pages/Question_Type.jsx (1)
Question_Type(6-123)
🔇 Additional comments (1)
eduaid_web/src/App.js (1)
17-17: Route wiring for /google-form looks correctThe route is properly registered and should work with HashRouter.
| {error && <div className="mt-4 text-red-400">{error}</div>} | ||
| {loading && ( | ||
| <div className="mt-8 text-white text-lg animate-pulse"> | ||
| Loading... | ||
| </div> | ||
| )} | ||
| {iframeUrl && ( | ||
| <div className="mt-8 w-full max-w-3xl"> | ||
| <iframe | ||
| src={iframeUrl} | ||
| width="100%" | ||
| height="800px" | ||
| className="rounded-lg shadow-lg border-0" | ||
| title="Google Doc/Form" | ||
| onLoad={() => setLoading(false)} // stop loader when iframe loads | ||
| ></iframe> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Avoid spinner hang; add iframe onError and a timeout fallback
If embedding is blocked (X-Frame-Options) or requires auth, onLoad may not fire, leaving the loader spinning.
Apply this diff to improve UX:
<iframe
src={iframeUrl}
width="100%"
height="800px"
className="rounded-lg shadow-lg border-0"
title="Google Doc/Form"
- onLoad={() => setLoading(false)}
+ onLoad={() => setLoading(false)}
+ onError={() => {
+ setLoading(false);
+ setError("Unable to load. The link may require access or disallow embedding.");
+ }}
></iframe>Optionally, add a fallback timeout after setting iframeUrl:
// right after setIframeUrl(...)
setTimeout(() => {
if (loading) {
setLoading(false);
setError("Taking too long to load. Please check sharing/embedding settings.");
}
}, 8000);🤖 Prompt for AI Agents
In eduaid_web/src/pages/Google_doc.jsx around lines 102 to 119, the iframe
loader can hang if embedding is blocked or auth is required because onLoad never
fires; add an onError handler on the iframe that calls setLoading(false) and
setError with a helpful message, and implement a timeout fallback right after
setIframeUrl that, after ~8000ms, checks if loading is still true and if so
calls setLoading(false) and setError("Taking too long to load. Please check
sharing/embedding settings."); ensure the timeout is cleared when the iframe
loads successfully (or when component unmounts) to avoid leaks.
| height="800px" | ||
| className="rounded-lg shadow-lg border-0" | ||
| title="Google Doc/Form" | ||
| onLoad={() => setLoading(false)} // stop loader when iframe loads |
There was a problem hiding this comment.
Fix JSX syntax: inline “//” comments aren’t valid inside JSX attributes
This will fail compilation.
Apply this diff:
- onLoad={() => setLoading(false)} // stop loader when iframe loads
+ onLoad={() => setLoading(false)}📝 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.
| onLoad={() => setLoading(false)} // stop loader when iframe loads | |
| onLoad={() => setLoading(false)} |
🤖 Prompt for AI Agents
In eduaid_web/src/pages/Google_doc.jsx around line 116, the JSX attribute
contains an inline "//" comment which is invalid and breaks compilation; remove
the inline "//" from the attribute and either place the comment as a JS line
comment immediately above the JSX element or convert it to a JSX comment (/* ...
*/) inside the element, and ensure the onLoad attribute remains as onLoad={() =>
setLoading(false)} without the inline // comment.
| import { div, Link } from "react-router-dom"; | ||
| import { useNavigate } from "react-router-dom"; |
There was a problem hiding this comment.
Fix invalid import: react-router-dom does not export “div”
This will cause a build error.
Apply this diff:
-import { div, Link } from "react-router-dom";
-import { useNavigate } from "react-router-dom";
+import { Link, useNavigate } from "react-router-dom";📝 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 { div, Link } from "react-router-dom"; | |
| import { useNavigate } from "react-router-dom"; | |
| import { Link, useNavigate } from "react-router-dom"; |
🤖 Prompt for AI Agents
In eduaid_web/src/pages/Question_Type.jsx around lines 4-5, remove the invalid
named import "div" from react-router-dom and consolidate imports; replace the
two imports with a single correct import that pulls Link and useNavigate from
"react-router-dom" (and ensure any JSX uses plain <div> elements, not an
imported div component).
| const handleOptionClick = (option) => { | ||
| setSelectedOption(option); | ||
| handleSaveToLocalStorage(); | ||
| }; | ||
|
|
||
| const handleSaveToLocalStorage = () => { |
There was a problem hiding this comment.
Avoid stale state: don’t navigate immediately after setSelectedOption
setState is async; calling handleSaveToLocalStorage right after may read the previous value and do nothing.
Apply this diff to make selection and navigation a two‑step flow (click option, then “Fire Up”):
- const handleOptionClick = (option) => {
- setSelectedOption(option);
- handleSaveToLocalStorage();
- };
+ const handleOptionClick = (option) => {
+ setSelectedOption(option);
+ };Alternatively, if you want immediate navigation on selection, pass the option into the handler:
- const handleOptionClick = (option) => {
- setSelectedOption(option);
- handleSaveToLocalStorage();
- };
+ const handleOptionClick = (option) => {
+ setSelectedOption(option);
+ handleSaveToLocalStorage(option);
+ };
- const handleSaveToLocalStorage = () => {
- if (selectedOption) {
- localStorage.setItem("selectedQuestionType", selectedOption);
- if(selectedOption === "dynamic_form"){
+ const handleSaveToLocalStorage = (opt) => {
+ const chosen = opt ?? selectedOption;
+ if (chosen) {
+ localStorage.setItem("selectedQuestionType", chosen);
+ if (chosen === "dynamic_form") {
navigate("/google-form");
- return;
- }else{
+ } else {
navigate("/input");
- }
+ }
}
};📝 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.
| const handleOptionClick = (option) => { | |
| setSelectedOption(option); | |
| handleSaveToLocalStorage(); | |
| }; | |
| const handleSaveToLocalStorage = () => { | |
| const handleOptionClick = (option) => { | |
| setSelectedOption(option); | |
| }; | |
| const handleSaveToLocalStorage = (opt) => { | |
| const chosen = opt ?? selectedOption; | |
| if (chosen) { | |
| localStorage.setItem("selectedQuestionType", chosen); | |
| if (chosen === "dynamic_form") { | |
| navigate("/google-form"); | |
| } else { | |
| navigate("/input"); | |
| } | |
| } | |
| }; |
🤖 Prompt for AI Agents
In eduaid_web/src/pages/Question_Type.jsx around lines 9-14, the code calls
handleSaveToLocalStorage immediately after setSelectedOption which can read
stale state because setState is async; change to either: (A) make selection a
two-step flow by removing the immediate call and only setSelectedOption on
click, then add a separate “Fire Up” button that reads selectedOption and calls
handleSaveToLocalStorage/navigation; or (B) if you want immediate navigation on
selection, change handleOptionClick to pass the clicked option into the
save/navigation logic (use the option parameter to save and navigate instead of
reading selectedOption) so you rely on the fresh value rather than the async
state.
|
can you please review @yatikakain |
1 participant
Resolved Issue #295
Implemented Dynamic Google Form Integration allowing users/admins to load Google Docs or Forms dynamically.
Recording.2025-09-23.221619.1.mp4
Summary by CodeRabbit