Upgrade premium feature buttons to show subscription prompt#82
Upgrade premium feature buttons to show subscription prompt#82polvallverdu wants to merge 2 commits intomainfrom
Conversation
Co-authored-by: polv <polv@uoc.edu>
|
Cursor Agent can help with this pull request. Just |
|
WalkthroughImplements a premium lock for voice input in ChatMessageInput by conditionally rendering the mic control based on subscription status. Introduces a new PremiumLockPopover Svelte component and exports it via the premium-badge index. No changes to exported functions beyond adding the new component. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ChatMessageInput
participant PremiumWrapper
participant PremiumLockPopover
participant VoiceService as voiceMessageService
participant Billing as /settings/billing
User->>ChatMessageInput: Open chat input
ChatMessageInput->>PremiumWrapper: Render voice input area
alt isUserSubscribed
User->>ChatMessageInput: Click mic
ChatMessageInput->>VoiceService: start/handle voice input (if not loading/error)
else not subscribed
ChatMessageInput->>PremiumLockPopover: Render locked mic trigger
User->>PremiumLockPopover: Click locked mic
PremiumLockPopover-->>User: Show "Premium feature" popover
User->>PremiumLockPopover: Click Subscribe
PremiumLockPopover->>Billing: Navigate to subscribeHref or default
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
✨ 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| <MicIcon class="h-4 w-4" /> | ||
| </Button> | ||
| </PremiumLockPopover> | ||
| {/if} |
There was a problem hiding this comment.
Bug: Voice Button UX Issues for Non-Subscribers
The voice input button for non-subscribed users, wrapped by PremiumLockPopover, lacks proper disabled state logic. Unlike the subscribed user's button, it is not disabled when the voice feature is unavailable (e.g., during loading, in unsupported browsers, or if the voice service has errors). This allows users to click the button and see the subscription popover even when the feature is technically unavailable, leading to poor UX.
Additionally, for non-subscribed users, both PremiumWrapper (displaying a badge) and PremiumLockPopover are rendered for the voice input button. This creates redundant premium messaging (a badge and a popover), causing UI clutter and potential confusion. The PremiumWrapper should be removed or its logic adjusted when PremiumLockPopover is used.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/lib/components/chatInput/ChatMessageInput.svelte (2)
48-48: Consolidate imports from the same module (nit)Importing from the same path twice is unnecessary. Combine with the existing import at Line 42 for consistency.
Apply this change:
- import { PremiumLockPopover } from "../ui/premium-badge";And update the earlier import (Line 42) to include both:
import { PremiumWrapper, PremiumLockPopover } from "../ui/premium-badge";
489-503: Improve subscription UX by preserving return pathPass a
subscribeHrefincluding areturnToquery so users are routed back to their chat after subscribing.-<PremiumLockPopover> +<PremiumLockPopover + subscribeHref={`/settings/billing?returnTo=${encodeURIComponent(page.url.pathname + page.url.search)}`} +> <Button variant="secondary" size="icon" aria-label="Voice input (Premium feature)" title="Voice input is a Premium feature"> <MicIcon class="h-4 w-4" /> </Button> </PremiumLockPopover>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/components/chatInput/ChatMessageInput.svelte(2 hunks)src/lib/components/ui/premium-badge/PremiumLockPopover.svelte(1 hunks)src/lib/components/ui/premium-badge/index.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/lib/components/chatInput/ChatMessageInput.svelte (1)
src/lib/client/services/voiceMessageService.svelte.ts (1)
VoiceMessageService(58-520)
⏰ 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: Cursor Bugbot
- GitHub Check: build-and-push
- GitHub Check: Lint
🔇 Additional comments (4)
src/lib/components/ui/premium-badge/PremiumLockPopover.svelte (3)
7-16: Bindable open prop is correctly wired for two-way controlUse of Snippet for children and
$bindable(false)onopenaligns with Svelte 5 runes and allows external control viabind:open. LGTM.
18-22: Verify PopoverTrigger composition to avoid nested interactive elementsIf
children()renders a Button (as in ChatMessageInput), ensure your PopoverTrigger implementation composes the child element (e.g., “asChild”-style) rather than rendering its own button. Otherwise you risk a button inside a button, which breaks a11y and events.Would you confirm
../popover’s Trigger composes its child instead of nesting interactive elements?
31-34: Check Button’s href behavior; prefer client-side navigation if supportedIf your Button supports
hrefby rendering an anchor under the hood, this is fine. If not, consider using your router’s navigation method (or a dedicated Link component) for SPA navigation to billing, preserving app state.Can you confirm
../buttonsupportshref(anchor rendering and proper a11y/ARIA)? If not, I can suggest an alternative.src/lib/components/ui/premium-badge/index.ts (1)
3-3: Export looks goodPublicly re-exporting
PremiumLockPopoveris consistent with existing index patterns.
| <Button | ||
| variant="secondary" | ||
| size="icon" | ||
| disabled={loading || !browser || voiceMessageService.state === "error"} | ||
| onclick={startRecording} | ||
| > | ||
| <MicIcon class="h-4 w-4" /> | ||
| </Button> | ||
| {:else} | ||
| <PremiumLockPopover> | ||
| <Button variant="secondary" size="icon"> | ||
| <MicIcon class="h-4 w-4" /> | ||
| </Button> | ||
| </PremiumLockPopover> | ||
| {/if} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessible labels for icon-only buttons
Icon-only buttons should have an accessible name. Add aria-label (and optionally title) to both the subscriber mic and the locked mic trigger.
-<Button
+<Button
+ aria-label="Start voice input"
variant="secondary"
size="icon"
disabled={loading || !browser || voiceMessageService.state === "error"}
onclick={startRecording}
>
<MicIcon class="h-4 w-4" />
</Button>-<PremiumLockPopover>
+<PremiumLockPopover>
<Button variant="secondary" size="icon"
+ aria-label="Voice input (Premium feature)"
+ title="Voice input is a Premium feature"
>
<MicIcon class="h-4 w-4" />
</Button>
</PremiumLockPopover>📝 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.
| <Button | |
| variant="secondary" | |
| size="icon" | |
| disabled={loading || !browser || voiceMessageService.state === "error"} | |
| onclick={startRecording} | |
| > | |
| <MicIcon class="h-4 w-4" /> | |
| </Button> | |
| {:else} | |
| <PremiumLockPopover> | |
| <Button variant="secondary" size="icon"> | |
| <MicIcon class="h-4 w-4" /> | |
| </Button> | |
| </PremiumLockPopover> | |
| {/if} | |
| <Button | |
| aria-label="Start voice input" | |
| variant="secondary" | |
| size="icon" | |
| disabled={loading || !browser || voiceMessageService.state === "error"} | |
| onclick={startRecording} | |
| > | |
| <MicIcon class="h-4 w-4" /> | |
| </Button> | |
| {:else} | |
| <PremiumLockPopover> | |
| <Button variant="secondary" size="icon" | |
| aria-label="Voice input (Premium feature)" | |
| title="Voice input is a Premium feature" | |
| > | |
| <MicIcon class="h-4 w-4" /> | |
| </Button> | |
| </PremiumLockPopover> | |
| {/if} |
🤖 Prompt for AI Agents
In src/lib/components/chatInput/ChatMessageInput.svelte around lines 489 to 503,
the icon-only Mic buttons lack accessible names; add aria-label attributes (and
optionally title attributes) to both Button components — for the
active/subscriber button use a descriptive label like "Start voice recording"
(or "Start recording") and for the locked/premium button use "Voice messaging
locked" or "Unlock voice messages" — add these attributes directly on the Button
elements and ensure the labels make sense with the disabled state.
2 participants
Make the locked voice input button open a popover with subscription options instead of being disabled.
Summary by CodeRabbit