Added screen share consent modal to trackpad route with i18n support#364
Open
Aryan-en wants to merge 1 commit into
Open
Added screen share consent modal to trackpad route with i18n support#364Aryan-en wants to merge 1 commit into
Aryan-en wants to merge 1 commit into
Conversation
… so issue raised by imxade, it will always ask for Screen sharing person on browser with matching UI
Contributor
WalkthroughAdds a localized full-screen screen-share consent component and updates the trackpad route to withhold the WebRTC token until consent is granted, then render the existing streaming flow. ChangesScreen-share consent
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant TrackpadRoute
participant ScreenShareConsent
participant useWebRtcStream
participant ScreenMirror
TrackpadRoute->>ScreenShareConsent: render before consent
ScreenShareConsent->>TrackpadRoute: call onAllow
TrackpadRoute->>useWebRtcStream: provide token
useWebRtcStream->>ScreenMirror: provide stream
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
Introduces a screen-sharing consent step in the /trackpad route to ensure users explicitly opt in before starting screen mirroring/control, and adds i18n strings to support the new UI.
Changes:
- Added a screen share consent overlay to the trackpad page and gated WebRTC session initiation behind user consent.
- Added new i18n translation keys for the consent modal copy and actions.
- Cleaned up
package-lock.jsonby removing extraneous entries.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/utils/i18n.ts |
Adds screenShareConsent translation strings used by the new consent UI. |
src/routes/trackpad.tsx |
Introduces consent state and uses it to gate WebRTC stream setup in the trackpad route. |
src/components/Trackpad/ScreenShareConsent.tsx |
New consent modal component shown before screen share begins. |
package-lock.json |
Removes extraneous dependency entries from the lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+46
to
51
| const [screenShareConsented, setScreenShareConsented] = useState(false) | ||
| const { status, send, sendCombo } = useRemoteConnection() | ||
| const { trackActive, videoStream, error, errorHandle, reconnect } = | ||
| useWebRtcStream({ | ||
| token, | ||
| token: screenShareConsented ? token : null, | ||
| }) |
Comment on lines
+15
to
+36
| <div className="absolute inset-0 flex items-center justify-center bg-black overflow-hidden select-none touch-none"> | ||
| <div className="w-full h-full place-content-center p-6 bg-base-300 shadow-2xl flex flex-col items-center text-center gap-6 duration-300"> | ||
| <div className="relative flex items-center justify-center w-16 h-16 rounded-full bg-primary/10 text-primary"> | ||
| {denied ? ( | ||
| <ShieldAlert className="w-8 h-8" /> | ||
| ) : ( | ||
| <ScreenShare className="w-8 h-8" /> | ||
| )} | ||
| </div> | ||
|
|
||
| <div className="space-y-2"> | ||
| <h3 className="text-xl font-bold text-base-content"> | ||
| {denied | ||
| ? t("screenShareConsent", "deniedTitle") | ||
| : t("screenShareConsent", "title")} | ||
| </h3> | ||
| <p className="text-sm text-base-content/70 max-w-sm px-2"> | ||
| {denied | ||
| ? t("screenShareConsent", "deniedDescription") | ||
| : t("screenShareConsent", "description")} | ||
| </p> | ||
| </div> |
![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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Trackpad/ScreenShareConsent.tsx`:
- Around line 14-61: Update the consent overlay in ScreenShareConsent to expose
modal semantics with role="dialog", aria-modal="true", and an accessible label
tied to the dialog heading. Make the overlay focusable and move focus to it when
the component mounts, using the existing component lifecycle and a ref while
preserving the current button interactions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c65a2e47-32b1-4c5a-a473-742f3e1d0fdb
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
src/components/Trackpad/ScreenShareConsent.tsxsrc/routes/trackpad.tsxsrc/utils/i18n.ts
Comment on lines
+14
to
+61
| return ( | ||
| <div className="absolute inset-0 flex items-center justify-center bg-black overflow-hidden select-none touch-none"> | ||
| <div className="w-full h-full place-content-center p-6 bg-base-300 shadow-2xl flex flex-col items-center text-center gap-6 duration-300"> | ||
| <div className="relative flex items-center justify-center w-16 h-16 rounded-full bg-primary/10 text-primary"> | ||
| {denied ? ( | ||
| <ShieldAlert className="w-8 h-8" /> | ||
| ) : ( | ||
| <ScreenShare className="w-8 h-8" /> | ||
| )} | ||
| </div> | ||
|
|
||
| <div className="space-y-2"> | ||
| <h3 className="text-xl font-bold text-base-content"> | ||
| {denied | ||
| ? t("screenShareConsent", "deniedTitle") | ||
| : t("screenShareConsent", "title")} | ||
| </h3> | ||
| <p className="text-sm text-base-content/70 max-w-sm px-2"> | ||
| {denied | ||
| ? t("screenShareConsent", "deniedDescription") | ||
| : t("screenShareConsent", "description")} | ||
| </p> | ||
| </div> | ||
| <div className="divider my-0 opacity-40" /> | ||
| <div className="space-y-3 w-full max-w-xs"> | ||
| <button | ||
| type="button" | ||
| onClick={onAllow} | ||
| className="btn btn-block btn-primary gap-2 shadow-lg shadow-primary/20 hover:scale-[1.02] active:scale-[0.98] transition-all duration-200" | ||
| > | ||
| <ScreenShare className="w-4 h-4" /> | ||
| {denied | ||
| ? t("screenShareConsent", "tryAgain") | ||
| : t("screenShareConsent", "allow")} | ||
| </button> | ||
| {!denied && ( | ||
| <button | ||
| type="button" | ||
| onClick={() => setDenied(true)} | ||
| className="btn btn-block btn-ghost" | ||
| > | ||
| {t("screenShareConsent", "deny")} | ||
| </button> | ||
| )} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add ARIA dialog semantics and focus management to the consent overlay.
The overlay functions as a modal dialog but lacks role="dialog", aria-modal="true", and an accessible label. Screen readers may not announce it as a dialog, and focus is not moved to the overlay on mount, leaving keyboard users on the underlying TouchArea. As per path instructions, accessibility blockers that prevent task completion (e.g., non-navigable modals) must be addressed.
♿ Proposed fix for ARIA and focus management
import { useState } from "react"
+import { useEffect, useRef } from "react"
import { ScreenShare, ShieldAlert } from "lucide-react"
import { t } from "../../utils/i18n"
interface ScreenShareConsentProps {
onAllow: () => void
}
export const ScreenShareConsent = ({ onAllow }: ScreenShareConsentProps) => {
const [denied, setDenied] = useState(false)
+ const allowButtonRef = useRef<HTMLButtonElement>(null)
+
+ useEffect(() => {
+ allowButtonRef.current?.focus()
+ }, [])
return (
- <div className="absolute inset-0 flex items-center justify-center bg-black overflow-hidden select-none touch-none">
+ <div
+ className="absolute inset-0 flex items-center justify-center bg-black overflow-hidden select-none touch-none"
+ role="dialog"
+ aria-modal="true"
+ aria-label={t("screenShareConsent", "title")}
+ >
<div className="w-full h-full place-content-center p-6 bg-base-300 shadow-2xl flex flex-col items-center text-center gap-6 duration-300">
<div className="relative flex items-center justify-center w-16 h-16 rounded-full bg-primary/10 text-primary">
{denied ? (
<ShieldAlert className="w-8 h-8" />
) : (
<ScreenShare className="w-8 h-8" />
)}
</div>
<div className="space-y-2">
<h3 className="text-xl font-bold text-base-content">
{denied
? t("screenShareConsent", "deniedTitle")
: t("screenShareConsent", "title")}
</h3>
<p className="text-sm text-base-content/70 max-w-sm px-2">
{denied
? t("screenShareConsent", "deniedDescription")
: t("screenShareConsent", "description")}
</p>
</div>
<div className="divider my-0 opacity-40" />
<div className="space-y-3 w-full max-w-xs">
<button
+ ref={allowButtonRef}
type="button"
onClick={onAllow}
className="btn btn-block btn-primary gap-2 shadow-lg shadow-primary/20 hover:scale-[1.02] active:scale-[0.98] transition-all duration-200"
>
<ScreenShare className="w-4 h-4" />
{denied
? t("screenShareConsent", "tryAgain")
: t("screenShareConsent", "allow")}
</button>
{!denied && (
<button
type="button"
onClick={() => setDenied(true)}
className="btn btn-block btn-ghost"
>
{t("screenShareConsent", "deny")}
</button>
)}
</div>
</div>
</div>
)
}As per path instructions, accessibility blockers that prevent task completion such as non-navigable modals must be addressed.
📝 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.
Suggested change
| return ( | |
| <div className="absolute inset-0 flex items-center justify-center bg-black overflow-hidden select-none touch-none"> | |
| <div className="w-full h-full place-content-center p-6 bg-base-300 shadow-2xl flex flex-col items-center text-center gap-6 duration-300"> | |
| <div className="relative flex items-center justify-center w-16 h-16 rounded-full bg-primary/10 text-primary"> | |
| {denied ? ( | |
| <ShieldAlert className="w-8 h-8" /> | |
| ) : ( | |
| <ScreenShare className="w-8 h-8" /> | |
| )} | |
| </div> | |
| <div className="space-y-2"> | |
| <h3 className="text-xl font-bold text-base-content"> | |
| {denied | |
| ? t("screenShareConsent", "deniedTitle") | |
| : t("screenShareConsent", "title")} | |
| </h3> | |
| <p className="text-sm text-base-content/70 max-w-sm px-2"> | |
| {denied | |
| ? t("screenShareConsent", "deniedDescription") | |
| : t("screenShareConsent", "description")} | |
| </p> | |
| </div> | |
| <div className="divider my-0 opacity-40" /> | |
| <div className="space-y-3 w-full max-w-xs"> | |
| <button | |
| type="button" | |
| onClick={onAllow} | |
| className="btn btn-block btn-primary gap-2 shadow-lg shadow-primary/20 hover:scale-[1.02] active:scale-[0.98] transition-all duration-200" | |
| > | |
| <ScreenShare className="w-4 h-4" /> | |
| {denied | |
| ? t("screenShareConsent", "tryAgain") | |
| : t("screenShareConsent", "allow")} | |
| </button> | |
| {!denied && ( | |
| <button | |
| type="button" | |
| onClick={() => setDenied(true)} | |
| className="btn btn-block btn-ghost" | |
| > | |
| {t("screenShareConsent", "deny")} | |
| </button> | |
| )} | |
| </div> | |
| </div> | |
| </div> | |
| ) | |
| import { useEffect, useRef, useState } from "react" | |
| import { ScreenShare, ShieldAlert } from "lucide-react" | |
| import { t } from "../../utils/i18n" | |
| interface ScreenShareConsentProps { | |
| onAllow: () => void | |
| } | |
| export const ScreenShareConsent = ({ onAllow }: ScreenShareConsentProps) => { | |
| const [denied, setDenied] = useState(false) | |
| const allowButtonRef = useRef<HTMLButtonElement>(null) | |
| useEffect(() => { | |
| allowButtonRef.current?.focus() | |
| }, []) | |
| return ( | |
| <div | |
| className="absolute inset-0 flex items-center justify-center bg-black overflow-hidden select-none touch-none" | |
| role="dialog" | |
| aria-modal="true" | |
| aria-label={t("screenShareConsent", "title")} | |
| > | |
| <div className="w-full h-full place-content-center p-6 bg-base-300 shadow-2xl flex flex-col items-center text-center gap-6 duration-300"> | |
| <div className="relative flex items-center justify-center w-16 h-16 rounded-full bg-primary/10 text-primary"> | |
| {denied ? ( | |
| <ShieldAlert className="w-8 h-8" /> | |
| ) : ( | |
| <ScreenShare className="w-8 h-8" /> | |
| )} | |
| </div> | |
| <div className="space-y-2"> | |
| <h3 className="text-xl font-bold text-base-content"> | |
| {denied | |
| ? t("screenShareConsent", "deniedTitle") | |
| : t("screenShareConsent", "title")} | |
| </h3> | |
| <p className="text-sm text-base-content/70 max-w-sm px-2"> | |
| {denied | |
| ? t("screenShareConsent", "deniedDescription") | |
| : t("screenShareConsent", "description")} | |
| </p> | |
| </div> | |
| <div className="divider my-0 opacity-40" /> | |
| <div className="space-y-3 w-full max-w-xs"> | |
| <button | |
| ref={allowButtonRef} | |
| type="button" | |
| onClick={onAllow} | |
| className="btn btn-block btn-primary gap-2 shadow-lg shadow-primary/20 hover:scale-[1.02] active:scale-[0.98] transition-all duration-200" | |
| > | |
| <ScreenShare className="w-4 h-4" /> | |
| {denied | |
| ? t("screenShareConsent", "tryAgain") | |
| : t("screenShareConsent", "allow")} | |
| </button> | |
| {!denied && ( | |
| <button | |
| type="button" | |
| onClick={() => setDenied(true)} | |
| className="btn btn-block btn-ghost" | |
| > | |
| {t("screenShareConsent", "deny")} | |
| </button> | |
| )} | |
| </div> | |
| </div> | |
| </div> | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Trackpad/ScreenShareConsent.tsx` around lines 14 - 61, Update
the consent overlay in ScreenShareConsent to expose modal semantics with
role="dialog", aria-modal="true", and an accessible label tied to the dialog
heading. Make the overlay focusable and move focus to it when the component
mounts, using the existing component lifecycle and a ref while preserving the
current button interactions.
Source: Path instructions
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
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.
… so issue raised by imxade, it will always ask for Screen sharing person on browser with matching UI
Addressed Issues:
Fixes #(issue number)
Description
Screenshots/Recordings:
Functional Verification
Screen Mirror
Authentication
Basic Gestures
One-finger tap: Verified as Left Click.
Two-finger tap: Verified as Right Click.
Click and drag: Verified selection behavior.
Pinch to zoom: Verified zoom functionality (if applicable).
Modes & Settings
Cursor mode: Cursor moves smoothly and accurately.
Scroll mode: Page scrolls as expected.
Sensitivity: Verified changes in cursor speed/sensitivity settings.
Copy and Paste: Verified both Copy and Paste functionality.
Invert Scrolling: Verified scroll direction toggles correctly.
Advanced Input
Key combinations: Verified "hold" behavior for modifiers (e.g., Ctrl+C) and held keys are shown in buffer.
Keyboard input: Verified Space, Backspace, and Enter keys work correctly.
Glide typing: Verified path drawing and text output.
Voice input: Verified speech-to-text functionality for full sentences.
Backspace doesn't send the previous input.
Any other gesture or input behavior introduced:
Additional Notes:
Checklist
My PR addresses a single issue, fixes a single bug or makes a single improvement.
My code follows the project's code style and conventions
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
If applicable, I have made corresponding changes or additions to the documentation
If applicable, I have made corresponding changes or additions to tests
My changes generate no new warnings or errors
I have joined the and I will share a link to this PR with the project maintainers there
I have read the
Once I submit my PR, CodeRabbit AI will automatically review it and I will address CodeRabbit's comments.
Incase of UI change I've added a demo video.
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.
Summary by CodeRabbit