Skip to content

Conversation

cpettet
Copy link
Contributor

@cpettet cpettet commented Aug 6, 2025

What is this PR doing?

Subscribers should be able to Scroll and Zoom on Content Sharing.

How should this be manually tested?

Enter to the app and share screen. Then, open another tab with a subscriber so as to see the screen shared. You should see a zoom icon on the right down part of the screen sharing.

What are the relevant tickets?

A maintainer will add this ticket number.

Resolves VIDSOL-34

Checklist

[✅] Branch is based on develop (not main).
[ ] Resolves a Known Issue.
[ ] If yes, did you remove the item from the docs/KNOWN_ISSUES.md?
[ ] Resolves an item reported in Issues.
If yes, which issue? Issue Number?

@github-actions github-actions bot changed the base branch from main to develop August 6, 2025 18:27
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
78.1% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@VZaphod VZaphod requested a review from Copilot October 3, 2025 12:09
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements scroll/zoom functionality for screenshare components by creating a dedicated screenshare video tile with zoom and pan capabilities. The implementation replaces conditional logic in the existing Subscriber component with a specialized ScreenshareVideoTile component.

  • Creates new ScreenshareVideoTile component with mouse wheel zoom and drag-to-pan functionality
  • Adds ZoomIndicator component to show current zoom level and provide reset functionality
  • Refactors Subscriber component to use ScreenshareVideoTile for screenshare content

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

File Description
frontend/src/components/Subscriber/Subscriber.tsx Refactored to use ScreenshareVideoTile for screenshare content, removing conditional UI logic
frontend/src/components/MeetingRoom/ZoomIndicator/ New component displaying zoom level percentage and reset button
frontend/src/components/MeetingRoom/ScreenshareVideoTile/ New specialized video tile with zoom/pan capabilities for screenshare content
Comments suppressed due to low confidence (1)

frontend/src/components/Subscriber/Subscriber.tsx:1

  • The isScreenshare prop is being removed from VideoTile but the line is still shown as removed. This suggests the prop may still be referenced in the VideoTile component definition and should be cleaned up there as well.
import { MouseEvent, ReactElement, useEffect, useRef, useState } from 'react';

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

VZaphod
VZaphod previously approved these changes Oct 3, 2025
Copy link

@VZaphod VZaphod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

behei-vonage
behei-vonage previously approved these changes Oct 7, 2025
Copy link
Contributor

@behei-vonage behei-vonage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

@dwivedisachin
Copy link
Contributor

Tested the PR:

  • Zoom not working on mobile (Safari iOS): Pinch and zoom is not working; suggest hiding the zoom icon on mobile.
  • Misleading tooltip: Zoom icon tooltip says “Zoom in” but clicking does nothing; should reflect actual behavior (e.g., “Fit to screen” or “100% zoom”).
  • Zoom icon interaction: When zoomed in, only the lens icon is clickable; the displayed zoom percentage is not interactive.
  • Inconsistent gestures across browsers: On Safari, pinch and zoom is not working, and scroll up and down gestures zoom in/out inconsistently; Chrome works with pinch and zomm and scroll both.
  • Lack of + / – controls: Users with a mouse cannot zoom easily; adding “+” and “–” buttons would improve usability.

@OscarFava OscarFava dismissed stale reviews from behei-vonage and VZaphod via 8a1965a October 10, 2025 11:17
Copy link

Copy link
Contributor

@behei-vonage behei-vonage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

@OscarFava OscarFava requested a review from VZaphod October 13, 2025 11:10
Copy link

@VZaphod VZaphod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants