Minor fixes 0811 (consent notification, floating button visibility) #1320
Conversation
📝 WalkthroughWalkthroughAdjusts multiple editor and toolbar UI components: FloatingButton and ListenButton visibility/consent logic now consider ongoing session and transcript presence; Share/New-note button visuals and placement updated; ShareButton export/email body and popover styles changed; locale PO files updated to match new source locations. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as EditorArea
participant FB as FloatingButton
participant SS as SessionStore
UI->>FB: Render()
FB->>SS: read ongoingSessionStatus, ongoingSessionId
FB->>SS: read session.enhanced_memo_html, isEnhancePending, session transcript/words
FB->>FB: compute isSessionInactive, hasTranscript, canEnhanceTranscript
alt enhanced_memo_html || isEnhancePending || canEnhanceTranscript
FB-->>UI: Render enhance button
else
FB-->>UI: Return null (do not show)
end
sequenceDiagram
participant NH as NoteHeader
participant LB as ListenButton
participant SS as SessionStore
NH->>LB: Render(sessionId)
LB->>SS: read ongoingActive, onboarding
LB->>SS: read session.words for sessionId
LB->>LB: evaluate ongoingActive matches sessionId && !onboarding && words.length === 0
alt conditions met
LB-->>NH: Show consent notification
else
LB-->>NH: Do not show consent
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic analysis
No issues found across 4 files. Review in cubic
![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 (3)
apps/desktop/src/components/editor-area/note-header/listen-button.tsx (1)
99-105: Prevent duplicate consent toasts; confirm toast deduplication by idThe effect will re-run on dependency changes. You pass a fixed toast id ("recording-consent-reminder"), which likely dedupes, but please verify the toast wrapper enforces id-based uniqueness to avoid repeated pop-ups.
If ids aren’t deduped by the UI toast, consider guarding with a ref so we toast at most once per mount:
- Expected behavior: Only a single consent toast shows during “running_active” while transcript is empty.
- If the wrapper doesn’t dedupe by id, add a ref guard:
Outside the effect:
const hasShownConsentRef = useRef(false);Inside the condition:
if (/* existing conditions */ && !hasShownConsentRef.current) { showConsentNotification(); hasShownConsentRef.current = true; }apps/desktop/src/components/editor-area/floating-button.tsx (2)
77-83: New visibility logic reads well; consider more precise naming for isSessionInactiveThe predicate also covers “different ongoing session id,” not just inactivity. Consider renaming to reduce ambiguity (e.g., isNotCurrentOngoingSession) for clarity.
191-195: Render guard inversion changes visibility semantics — please verify UXPreviously the button was hidden unless memo HTML was absent and not pending. Now it’s shown when any of the following is true: enhanced memo exists, enhancement is pending, or transcript exists for a non-current/inactive session. This looks intentional per PR objectives, but it’s a behavioral change. Please validate these flows:
- No transcript, not pending, no enhanced memo → button hidden (expected).
- Transcript exists and session is inactive/not current → button visible to allow enhancement (expected).
- Enhanced memo exists → button visible for raw/enhanced toggle and rerun (expected).
Also minor: coerce to boolean for readability and type clarity.
Apply this diff locally:
- const shouldShowButton = session.enhanced_memo_html || isEnhancePending || canEnhanceTranscript; + const shouldShowButton = Boolean(session.enhanced_memo_html) || isEnhancePending || canEnhanceTranscript;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
apps/desktop/src/components/editor-area/floating-button.tsx(2 hunks)apps/desktop/src/components/editor-area/note-header/listen-button.tsx(1 hunks)apps/desktop/src/locales/en/messages.po(7 hunks)apps/desktop/src/locales/ko/messages.po(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit Configuration File
**/*.{js,ts,tsx,rs}: 1. No error handling.
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/editor-area/note-header/listen-button.tsxapps/desktop/src/components/editor-area/floating-button.tsx
🔇 Additional comments (3)
apps/desktop/src/components/editor-area/note-header/listen-button.tsx (1)
95-96: Gating consent by transcript presence — LGTMUsing sessionWords to avoid showing consent once there’s already a transcript is the right call and aligns with the PR’s intent.
apps/desktop/src/locales/ko/messages.po (1)
271-274: PO source-location updates only — LGTMOnly reference anchors moved to match listen-button.tsx edits; no msgid/msgstr changes. Safe to merge.
Also applies to: 1120-1122, 1200-1202, 1284-1286, 1297-1299, 1438-1440, 1442-1444, 1487-1489
apps/desktop/src/locales/en/messages.po (1)
271-274: PO source-location updates only — LGTMSame as ko: purely metadata (source refs) updated for listen-button.tsx; translations unchanged.
Also applies to: 1120-1122, 1200-1202, 1283-1286, 1296-1299, 1437-1440, 1441-1444, 1486-1489
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src/components/toolbar/buttons/new-note-button.tsx (1)
37-37: Button styling is consistent but consider design system alignment.The explicit
bg-transparent text-blackclasses ensure consistent button appearance. However, verify this aligns with your design system's button variants rather than using inline overrides.Consider using a dedicated button variant if this styling pattern is used elsewhere:
-className="hover:bg-neutral-200 bg-transparent text-black" +variant="toolbar" // or similar semantic variant
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/desktop/src/components/toolbar/bars/main-toolbar.tsx(1 hunks)apps/desktop/src/components/toolbar/buttons/new-note-button.tsx(1 hunks)apps/desktop/src/components/toolbar/buttons/share-button.tsx(7 hunks)apps/desktop/src/components/toolbar/buttons/transcript-panel-button.tsx(1 hunks)apps/desktop/src/locales/en/messages.po(9 hunks)apps/desktop/src/locales/ko/messages.po(9 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/components/toolbar/buttons/transcript-panel-button.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/locales/ko/messages.po
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit Configuration File
**/*.{js,ts,tsx,rs}: 1. No error handling.
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/toolbar/bars/main-toolbar.tsxapps/desktop/src/components/toolbar/buttons/new-note-button.tsxapps/desktop/src/components/toolbar/buttons/share-button.tsx
🧬 Code Graph Analysis (1)
apps/desktop/src/components/toolbar/bars/main-toolbar.tsx (1)
apps/desktop/src/components/toolbar/buttons/share-button.tsx (1)
ShareButton(28-31)
⏰ 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). (3)
- GitHub Check: zizmor
- GitHub Check: ci (macos, macos-latest)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (15)
apps/desktop/src/components/toolbar/bars/main-toolbar.tsx (1)
60-60: LGTM! ShareButton placement improvement.The ShareButton is now properly placed on the right side of the toolbar within the main window context, which aligns with the PR objectives to improve UI organization. The conditional rendering ensures it only appears when viewing a note in the main window.
apps/desktop/src/locales/en/messages.po (4)
1120-1122: Translation reference updated correctly.The "No Template (Default)" reference has been updated to line 519, which aligns with the source location changes.
1539-1544: New translation entry properly added with obsolete cleanup.The new "Toggle transcriptpanel" translation has been added with the correct reference (line 36), and the obsolete "Toggle widget panel" entry is properly marked with
#~. This follows proper PO file maintenance practices.
637-639: Translation reference is accurate – no change needed.Verified that the
<Trans>Create new note</Trans>string resides on line 46 of
apps/desktop/src/components/toolbar/buttons/new-note-button.tsx, matching the reference in
apps/desktop/src/locales/en/messages.po:637.
No mismatch detected; you can disregard the original comment.Likely an incorrect or invalid review comment.
271-273: Placeholder references are accurate
The.poentries at lines 271–273 (#: …listen-button.tsx:222,…:244,…:264) match the<Trans>calls inapps/desktop/src/components/editor-area/note-header/listen-button.tsxat lines 222, 244, and 264. No update to line numbers is needed.apps/desktop/src/components/toolbar/buttons/share-button.tsx (10)
7-7: Icon import change looks good.Changed from
Share2IcontoShare- verify this provides the desired visual appearance in the UI.
242-242: Dynamic background state for open popover.The conditional background class provides visual feedback when the popover is open, enhancing UX. The implementation is clean and follows React patterns.
245-245: Icon usage updated consistently.Updated to use the new
Shareicon component imported on line 7.
251-251: Popover positioning adjustment.The
sideOffset={7}provides better visual spacing between the trigger and popover content.
272-272: Card-style visual treatment applied consistently.The white background and border styling creates a clean card appearance for the direct action buttons.
276-276: Button styling maintains consistency.Explicit white background ensures consistent appearance across the button states.
299-303: Expandable option styling improved.The card-style treatment with consistent backgrounds and hover states provides better visual hierarchy. The conditional styling based on expanded state is well implemented.
315-315: Content container styling enhanced.The lighter border (
border-gray-100) and white background create proper visual separation for expanded content.
368-368: Export button styling updated.Changed from
bg-gray-800tobg-blackandtransition-colorstotransition-allfor more prominent styling and smoother transitions.
442-455: Enhanced email export body content.The email body now includes:
- A descriptive prefix
- Proper markdown conversion from HTML content
- A signature with the app URL
- Proper URL encoding
This provides a much richer email sharing experience.
Summary by cubic
Improved floating button visibility logic and updated consent notification behavior to prevent showing it when a transcript already exists.