fix: nicer messaging on mobile for screenshare. Fixes RED5DEV-1796. #656
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves the user experience for screen sharing functionality on mobile devices by adding informative modal messaging when screen share capture fails, which commonly occurs on mobile browsers that don't support this capability.
- Added a reusable modal utility function for displaying messages
- Enhanced error handling in screen share capture functions to show user-friendly messages
- Added automatic cleanup when screen sharing ends unexpectedly
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| static/script/red5pro-utils.js | Added showModal utility function for displaying modal dialogs |
| src/page/test/publishScreenShare/index.js | Enhanced error handling with modal messaging and added video track end event handler |
| src/page/test/publishScreenShare/index.html | Added modal CSS styling |
| src/page/test/castlabs/index.html | Added black background styling to video element |
| src/page/sm-test/publishStreamManagerProxyScreenShare/index.js | Enhanced error handling with modal messaging and added video track end event handler |
| src/page/sm-test/publishStreamManagerProxyScreenShare/index.html | Added modal CSS styling |
| src/page/sm-test/castlabsStreamManagerProxy/index.html | Added black background styling to video element |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| document.body.appendChild(div); | ||
| button.addEventListener('click', (event) => { | ||
| event.preventDefault(); | ||
| document.body.removeChild(div); |
There was a problem hiding this comment.
The removeChild call could throw an error if the element has already been removed from the DOM. Consider using div.remove() or checking if the parent exists before removal.
| document.body.removeChild(div); | |
| div.remove(); |
| window.showModal = (content) => { | ||
| const div = document.createElement('div'); | ||
| div.classList.add('modal'); | ||
| const container = document.createElement('div'); |
There was a problem hiding this comment.
The container element lacks CSS classes or styling which may cause layout issues. Consider adding appropriate CSS classes for proper modal structure.
| const container = document.createElement('div'); | |
| const container = document.createElement('div'); | |
| container.classList.add('modal-container'); |
| videoTrack.onended = async () => { | ||
| if (targetPublisher) { | ||
| unpublish(targetPublisher) |
There was a problem hiding this comment.
No null check for videoTrack before accessing onended. If getVideoTracks() returns an empty array, this will throw an error when trying to set onended on undefined.
| videoTrack.onended = async () => { | |
| if (targetPublisher) { | |
| unpublish(targetPublisher) | |
| if (videoTrack) { | |
| videoTrack.onended = async () => { | |
| if (targetPublisher) { | |
| unpublish(targetPublisher) | |
| } |
| videoTrack.onended = async () => { | ||
| if (targetPublisher) { | ||
| unpublish(targetPublisher) |
There was a problem hiding this comment.
No null check for videoTrack before accessing onended. If getVideoTracks() returns an empty array, this will throw an error when trying to set onended on undefined.
| videoTrack.onended = async () => { | |
| if (targetPublisher) { | |
| unpublish(targetPublisher) | |
| if (videoTrack) { | |
| videoTrack.onended = async () => { | |
| if (targetPublisher) { | |
| unpublish(targetPublisher) | |
| } |
2 participants
No description provided.