fix: minor changes to UI and logic on canvas #150
Conversation
WalkthroughThis pull request introduces a new environment variable ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant IdeologyTest
participant API
participant Navigation
User->>IdeologyTest: Submit test
IdeologyTest->>API: Fetch existing insights and answers
alt Insights exist and answers unchanged
IdeologyTest->>Navigation: Redirect to Insights page
else Insights missing or answers changed
IdeologyTest->>IdeologyTest: Calculate scores
IdeologyTest->>API: Create/Update insights
IdeologyTest->>Navigation: Redirect with updated insights
end
sequenceDiagram
participant App
participant ErudaProvider
participant Eruda
App->>ErudaProvider: Render children
ErudaProvider->>ErudaProvider: Check NEXT_PUBLIC_ENABLE_DEVTOOLS env variable
alt DevTools Enabled
ErudaProvider->>Eruda: Initialize eruda library
Eruda->>ErudaProvider: Confirm initialization
ErudaProvider->>App: Wrap children with Eruda
else DevTools Disabled
ErudaProvider->>App: Render children without Eruda
end
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
✅ Deploy Preview for lucent-florentine-971919 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
frontend/src/app/ideology-test/page.tsx (2)
72-72: SecuringoriginalAnswersassignment.
Storing the server-sourced answers inoriginalAnswersis correct to allow comparison. Just ensure no concurrency issues when multiple updates could overwriteoriginalAnswers.
175-189: Handling multiple scenarios for final result submission is well-structured.
- Redirect if no changes and insights exist.
- Create new insights if none exist.
- Update (PUT) if user's answers changed.
Consider returning to a consistent code path or refactoring these into smaller functions to keep the logic DRY, especially if new scenarios arise.
frontend/src/components/Canvas.tsx (3)
21-41: Font loading function is robust, but consider parallelizing.
The current approach ensures the font is fetched before continuing. You could potentially reduce wait time with aPromise.allapproach if you load images in parallel.
85-98: Canvas dimension setting is explicit but watch for high-resolution screens.
You might want to account forwindow.devicePixelRatioto provide crisper rendering on Retina/HiDPI displays.
99-105: Debug font rendering usage.
Including a prototype text measurement is fine for debugging. Consider removing or gating logs in production if this is only for troubleshooting.frontend/src/app/insights/page.tsx (1)
551-559: Consider using React state management instead of script injection.The current implementation uses script injection to emit modal state. Consider using React's state management or a context provider instead.
- <script - dangerouslySetInnerHTML={{ - __html: ` - window.dispatchEvent(new CustomEvent('modalState', { - detail: { isOpen: ${isModalOpen} } - })); - `, - }} - /> + <ModalStateProvider isOpen={isModalOpen} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
frontend/.env.example(1 hunks)frontend/package.json(1 hunks)frontend/src/app/ideology-test/page.tsx(7 hunks)frontend/src/app/insights/page.tsx(8 hunks)frontend/src/components/Canvas.tsx(8 hunks)frontend/src/providers/eruda-provider.tsx(1 hunks)frontend/src/providers/index.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Code Quality & Build
🔇 Additional comments (15)
frontend/src/app/ideology-test/page.tsx (5)
31-31: Good addition oforiginalAnswersstate.
Helps track the baseline answers for comparison, which is crucial for determining if the user has updated their responses.
53-58: Ensure consistent completion logic.
When the test status is"completed", the immediate redirect to Insights is fine. However, consider whether there's any scenario (e.g., user reopened the test page in a new tab) where partial data might need reconciling before redirect.
101-101: Preventing double submission is good.
Avoiding multiple simultaneous submissions reduces the risk of data inconsistencies or duplicate results. Confirm that other interactions (e.g., handleAnswer calls) also gracefully checkisSubmitting.
116-116: Early score calculation is clear and logical.
Computing the final scores before branching into scenarios helps ensure you don’t recalculate or produce partial updates.
146-155: Insight existence and answer change checks look solid.
This is an essential guard to avoid unnecessary writes. Confirm you have server-side checks to handle edge cases (e.g., stale data).frontend/src/components/Canvas.tsx (4)
9-11: New propsideologyandonLoadadd flexibility.
Being able to pass anideologystring and an optional callback is a clean separation of concerns.
277-288: Extra spacing and text box for ideology look good.
The gradient styling complements the UI. Just confirm that very long ideologies (e.g., long text) remain visible without clipping.
316-356: Closest match display is consistent with the layout.
The design uses a highlight box and subtle glow. This ensures brand consistency—nice approach.
358-389: CTA button design is appealing.
The spacing and text alignment appear correct. Great call on callingonLoad?.()after drawing completes, letting parent components respond to the finished render.frontend/src/providers/index.tsx (1)
15-15: Environment-based conditional is correct.
Switching from the old environment variable toNEXT_PUBLIC_ENABLE_DEVTOOLSis a clean approach. Make sure to document usage for other devs.frontend/src/providers/eruda-provider.tsx (1)
13-13:✅ Verification successful
LGTM! Environment-based control for development tools.
The addition of the environment variable check improves control over when development tools are initialized.
Verify the environment variable is properly set in your development environment:
🏁 Script executed:
#!/bin/bash # Check if NEXT_PUBLIC_ENABLE_DEVTOOLS is set in .env files echo "Checking .env files for NEXT_PUBLIC_ENABLE_DEVTOOLS..." fd -t f "^\.env" -x grep -l "NEXT_PUBLIC_ENABLE_DEVTOOLS"Length of output: 384
Below is a revised shell script that ensures hidden files are included in the search. Please run it to verify that your hidden .env files correctly set the NEXT_PUBLIC_ENABLE_DEVTOOLS variable:
LGTM! Environment-based control for development tools is a solid addition.
The change to add an environment variable check improves the conditional initialization of Eruda. However, please verify your setup by running the above script to confirm that hidden .env files (which typically contain environment configurations) include the NEXT_PUBLIC_ENABLE_DEVTOOLS variable.
🏁 Script executed:
[run_scripts] #!/bin/bash echo "Searching hidden .env files for NEXT_PUBLIC_ENABLE_DEVTOOLS using -H flag..." fd -H -t f "^\.env" -x grep -l "NEXT_PUBLIC_ENABLE_DEVTOOLS"Length of output: 369
LGTM! Environment-based control for development tools is well implemented. The change ensures Eruda is initialized only when the
NEXT_PUBLIC_ENABLE_DEVTOOLSvariable is set to"true". The updated verification script confirms that the variable is present in your.env.examplefile. Please ensure that your actual development configuration (e.g., in.env.localor similar) also setsNEXT_PUBLIC_ENABLE_DEVTOOLSappropriately.frontend/src/app/insights/page.tsx (2)
40-41: LGTM! Added loading states for better UX.The addition of separate loading states for canvas and Gemini analysis improves user experience by providing clear feedback.
113-154: LGTM! Separated Gemini API call for better code organization.Moving the Gemini API call to a separate effect improves code organization and maintainability.
frontend/.env.example (1)
23-26: LGTM! Well-documented environment variables.The addition of NODE_ENV and NEXT_PUBLIC_ENABLE_DEVTOOLS with clear comments improves configuration documentation.
frontend/package.json (1)
48-48: LGTM! Correctly moved eruda to devDependencies.Moving eruda to devDependencies is appropriate as it's a development tool.
Summary by CodeRabbit