Merged
Conversation
…emo-instruction-modal
…emo-instruction-modal
Contributor
📝 WalkthroughWalkthroughAdds a tips modal feature and integrates it into EditorArea. Introduces a modal component, types, and a service to programmatically show it. EditorArea now determines whether to show the modal after note enhancement using a helper and session/localStorage checks, and consumes an added thankYouSessionId from useHypr. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant EA as EditorArea
participant SH as shouldShowTipsModal
participant DB as dbCommands
participant LS as localStorage
participant SV as showTipsModal (service)
participant TM as TipsModal
U->>EA: Trigger enhance
EA->>EA: Enhancement succeeds
EA->>EA: If not onboarding, wait 1.2s
EA->>SH: shouldShowTipsModal(userId, thankYouSessionId)
SH->>DB: Query sessions/flags
SH->>LS: Read "tipsModalShown"
SH-->>EA: true/false
alt should show
EA->>SV: showTipsModal(userId)
SV->>TM: Render TipsModal (isOpen=true)
TM-->>U: Display slides
U-->>TM: Close/Complete
TM-->>SV: onClose
SV-->>EA: Promise resolved
EA->>LS: Set "tipsModalShown"=true
else skip
EA->>EA: Continue without modal
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/desktop/src/components/tips-modal/service.ts (1)
5-26: De‑dupe modal instances to avoid multiple concurrent tips modals.If
showTipsModalis called twice before the first closes, you’ll end up with stacked modals. Add a lightweight guard.Apply this diff:
export function showTipsModal(userId?: string): Promise<void> { return new Promise((resolve) => { + // Why: ensure only one tips modal at a time + const existing = document.querySelector('[data-hypr-tips-modal="true"]'); + if (existing) { + resolve(); + return; + } const modalDiv = document.createElement("div"); + modalDiv.setAttribute("data-hypr-tips-modal", "true"); document.body.appendChild(modalDiv); const root = createRoot(modalDiv); const handleClose = () => { root.unmount(); document.body.removeChild(modalDiv); resolve(); };Optional: Convert this file to .tsx and use JSX instead of
React.createElementfor consistency.apps/desktop/src/components/tips-modal/index.tsx (2)
67-87: Reduce console noise when userId is missing.Return early instead of logging every render where userId is falsy.
- useEffect(() => { - if (userId) { + useEffect(() => { + if (!userId) return; const events = [ "tips_modal_intro_show", "tips_modal_transcript_show", "tips_modal_transcription_show", "tips_modal_intelligence_show", ]; const event = events[currentSlide]; if (event) { analyticsCommands.event({ event, distinct_id: userId, }); } - } else { - console.error("no userId available for analytics"); - } + }, [currentSlide, userId]);
125-160: Flatten nested ternaries for slide media.Move image metadata into an array keyed by slide index and render from it. This lowers cognitive complexity and eases adding slides.
Example outside this block:
const mediaBySlide = [ { src: "/assets/waving.gif", alt: "Celebration animation", className: "w-48 h-36 object-contain rounded-md", style: {} }, { src: "/assets/transcript-edit.gif", alt: "Transcript editing demonstration", className: "w-full max-w-lg h-64 object-cover rounded-md", style: { objectPosition: "center top" } }, { src: "/assets/transcription-setting.gif", alt: "Transcription settings demonstration", className: "w-full max-w-lg h-64 object-cover rounded-md", style: { objectPosition: "center top" } }, { src: "/assets/intelligence-setting.gif", alt: "Intelligence settings demonstration", className: "w-full max-w-lg h-64 object-cover rounded-md", style: { objectPosition: "center top" } }, ];Then render:
const media = mediaBySlide[currentSlide]; <img src={media.src} alt={media.alt} className={media.className} style={media.style} />apps/desktop/src/components/editor-area/index.tsx (2)
33-64: Scope the “shown” flag per user to avoid cross-user bleed on shared devices.Use a user-scoped storage key.
async function shouldShowTipsModal( userId: string, onboardingSessionId: string, thankYouSessionId: string, ): Promise<boolean> { try { - const hasSeenTips = localStorage.getItem(TIPS_MODAL_SHOWN_KEY) === "true"; + const storageKey = `${TIPS_MODAL_SHOWN_KEY}:${userId}`; + const hasSeenTips = localStorage.getItem(storageKey) === "true"; if (hasSeenTips) { return false; }
190-202: Mark tips as shown per user and guard against double invocation.
- Persist the shown flag with user scoping (complements the earlier refactor).
- Optional: add a lightweight in‑module sentinel to prevent back‑to‑back timeouts from showing the modal twice.
Apply this diff:
- localStorage.setItem(TIPS_MODAL_SHOWN_KEY, "true"); + localStorage.setItem(`${TIPS_MODAL_SHOWN_KEY}:${userId}`, "true"); showTipsModal(userId);Optional sentinel outside this block:
// Why: avoid showing tips twice if multiple enhance completions race let tipsModalInFlight = false;And inside the timeout:
if (tipsModalInFlight) return; tipsModalInFlight = true; // ... after showTipsModal resolves: tipsModalInFlight = false;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
apps/desktop/public/assets/intelligence-setting.gifis excluded by!**/*.gifapps/desktop/public/assets/transcript-edit.gifis excluded by!**/*.gifapps/desktop/public/assets/transcription-setting.gifis excluded by!**/*.gifapps/desktop/public/assets/waving.gifis excluded by!**/*.gif
📒 Files selected for processing (4)
apps/desktop/src/components/editor-area/index.tsx(3 hunks)apps/desktop/src/components/tips-modal/index.tsx(1 hunks)apps/desktop/src/components/tips-modal/service.ts(1 hunks)apps/desktop/src/components/tips-modal/types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop/src/components/tips-modal/index.tsxapps/desktop/src/components/tips-modal/service.tsapps/desktop/src/components/tips-modal/types.tsapps/desktop/src/components/editor-area/index.tsx
🧬 Code graph analysis (4)
apps/desktop/src/components/tips-modal/index.tsx (3)
apps/desktop/src/components/tips-modal/types.ts (2)
TipSlide(7-10)TipsModalProps(1-5)packages/ui/src/components/ui/modal.tsx (4)
Modal(23-83)ModalBody(99-105)ModalTitle(130-136)ModalDescription(143-148)packages/ui/src/components/ui/button.tsx (1)
Button(37-89)
apps/desktop/src/components/tips-modal/service.ts (1)
apps/desktop/src/components/tips-modal/index.tsx (1)
TipsModal(30-211)
apps/desktop/src/components/tips-modal/types.ts (1)
apps/desktop/src/components/tips-modal/index.tsx (2)
TipsModalProps(213-213)TipSlide(213-213)
apps/desktop/src/components/editor-area/index.tsx (2)
apps/desktop/src/contexts/hypr.tsx (1)
useHypr(63-69)apps/desktop/src/components/tips-modal/service.ts (1)
showTipsModal(5-26)
⏰ 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). (2)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (4)
apps/desktop/src/components/tips-modal/types.ts (2)
1-5: Types look good and minimal.Props surface is concise and matches usage in the modal and service.
7-10: Slide type is appropriately scoped.Fits current needs; easy to extend later if images/ids are added.
apps/desktop/src/components/tips-modal/index.tsx (1)
30-66: Component structure and analytics hooks look solid.State reset on close/complete and event emissions are correctly scoped to userId.
apps/desktop/src/components/editor-area/index.tsx (1)
127-127: Resolved — useHypr exposes thankYouSessionIdHyprProvider sets thankYouSessionId and useHypr returns it; no change required.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.