#326: Solution for a Video Popup Modal by soonu-kedari · Pull Request #329 · sumn2u/learn-javascript

soonu-kedari · 2025-10-05T06:17:57Z

Below concepts used in the Example:
Modal: A container overlaying the page for focused content
Event Listeners: Detects clicks or key presses
Video Control: Using .play(), .pause(), .current Time to manage playback
CSS Overlay: Covers the page for modal background
Responsive Design: Adapts layout to fit different devices

vercel · 2025-10-05T06:18:01Z

@soonu-kedari is attempting to deploy a commit to the Suman Kunwar's projects Team on Vercel.

A member of the Team first needs to authorize it.

Copilot

Pull Request Overview

This PR implements a video popup modal component for displaying video trailers with a movie card interface. The solution includes HTML structure, CSS styling with responsive design, and JavaScript functionality for modal interactions and video controls.

Adds a complete video modal popup system with movie card UI
Implements responsive design for mobile and tablet devices
Includes JavaScript event handling for modal open/close and video playback controls

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
examples/video-popup/index.html	HTML structure with movie card and modal elements
examples/video-popup/style.css	CSS styling with responsive breakpoints and animations
examples/video-popup/script.js	JavaScript for modal interactions and video controls
examples/video-popup/README.md	Documentation explaining the implementation and features

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

Copilot · 2025-10-05T10:30:41Z

+  margin: 10px 0 5px;
+}
+
+.popup-title{


Missing space before opening brace. Should be .popup-title { to follow consistent CSS formatting.

Suggested change

.popup-title{

.popup-title {

Copilot · 2025-10-05T10:30:41Z

+/* Modal overlay */
+.modal {
+  position: fixed;
+  top: 0; left: 0;


[nitpick] CSS properties should be on separate lines for better readability and maintainability.

Suggested change

top: 0; left: 0;

top: 0;

left: 0;

Copilot · 2025-10-05T10:30:42Z

+The `index.html` file defines the structure of the webpage. It includes:
+
+#### **Key Elements**
+- `<div class="movie-container">` — Wraps all content like the movie thumbnail, title, description, and button.


The class name 'movie-container' doesn't match the actual implementation which uses 'movie-card'. This should be updated to reflect the correct class name.

Suggested change

- `<div class="movie-container">` — Wraps all content like the movie thumbnail, title, description, and button.

- `<div class="movie-card">` — Wraps all content like the movie thumbnail, title, description, and button.

Copilot · 2025-10-05T10:30:42Z

+- `<img>` — Displays the movie’s thumbnail or poster image.
+- `<h1>` — The movie title.
+- `<p>` — A short movie description or tagline.
+- `<button id="openModal">` — Button to trigger the modal popup.


The ID 'openModal' doesn't match the actual implementation which uses 'openModalBtn'. This should be updated to reflect the correct ID.

Suggested change

- `<button id="openModal">` — Button to trigger the modal popup.

- `<button id="openModalBtn">` — Button to trigger the modal popup.

Copilot · 2025-10-05T10:30:42Z

+   const btn = document.getElementById("openModal");
+   const span = document.getElementsByClassName("close")[0];
+   const video = document.getElementById("trailerVideo");


The example code uses incorrect element IDs and class names that don't match the actual implementation. Should use 'openModalBtn', 'closeModalBtn', and 'trailer' respectively.

Suggested change

const btn = document.getElementById("openModal");

const span = document.getElementsByClassName("close")[0];

const video = document.getElementById("trailerVideo");

const btn = document.getElementById("openModalBtn");

const span = document.getElementById("closeModalBtn");

const video = document.getElementById("trailer");

sumn2u · 2025-10-05T10:34:11Z

@soonu-kedari When you have time can you look into above comments?

soonu-kedari · 2025-10-05T11:03:25Z

@sumn2u Fixed the comments

Copilot

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

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

Copilot · 2025-10-05T11:47:11Z

+body {
+  font-family: Arial, sans-serif;
+  background: #ffffff;
+  color: #fff;


The body has a white background (#ffffff) but white text color (#fff), making the text invisible. This should be corrected to use a dark text color like #333 or change the background to a dark color.

Suggested change

color: #fff;

color: #333;

Copilot · 2025-10-05T11:47:12Z

+- `<p>` — A short movie description or tagline.
+- `<button id="openModalBtn">` — Button to trigger the modal popup.
+- `<div id="videoModal" class="modal">` — The modal container that holds the video player.
+- `<span class="close">&times;</span>` — The close icon (×) used to close the modal.


The documentation shows the close button as a <span> element with class 'close', but the actual implementation uses a <button> element with class 'close-btn' and id 'closeModalBtn'.

Suggested change

- `<span class="close">×</span>` — The close icon (×) used to close the modal.

- `<button class="close-btn" id="closeModalBtn">×</button>` — The close icon (×) used to close the modal.

Copilot · 2025-10-05T11:47:12Z

+   btn.onclick = function() {
+       modal.style.display = "flex";
+       video.play();
+   }
+   ```
+   When the **Play Trailer** button is clicked, this function makes the modal visible and starts playing the video.
+
+3. **Close Modal Functionality**
+   ```js
+   span.onclick = function() {
+       modal.style.display = "none";
+       video.pause();
+       video.currentTime = 0;
+   }
+   ```
+   When the close icon (×) is clicked, the modal hides, the video pauses, and the playback resets.
+
+4. **Outside Click Close**
+   ```js
+   window.onclick = function(event) {
+       if (event.target == modal) {
+           modal.style.display = "none";
+           video.pause();
+           video.currentTime = 0;
+       }
+   }


The documentation shows outdated code using onclick property and style.display, but the actual implementation uses addEventListener and CSS classes with classList.add('active').

Suggested change

btn.onclick = function() {

modal.style.display = "flex";

video.play();

}

```

When the **Play Trailer** button is clicked, this function makes the modal visible and starts playing the video.

3. **Close Modal Functionality**

```js

span.onclick = function() {

modal.style.display = "none";

video.pause();

video.currentTime = 0;

}

```

When the close icon (×) is clicked, the modal hides, the video pauses, and the playback resets.

4. **Outside Click Close**

```js

window.onclick = function(event) {

if (event.target == modal) {

modal.style.display = "none";

video.pause();

video.currentTime = 0;

}

}

btn.addEventListener('click', function() {

modal.classList.add('active');

video.play();

});

When the Play Trailer button is clicked, this function makes the modal visible and starts playing the video.

Close Modal Functionality

span.addEventListener('click', function() { modal.classList.remove('active'); video.pause(); video.currentTime = 0; });

When the close icon (×) is clicked, the modal hides, the video pauses, and the playback resets.

Outside Click Close

window.addEventListener('click', function(event) { if (event.target === modal) { modal.classList.remove('active'); video.pause(); video.currentTime = 0; } });

Copilot · 2025-10-05T11:47:12Z

+   btn.onclick = function() {
+       modal.style.display = "flex";
+       video.play();
+   }
+   ```
+   When the **Play Trailer** button is clicked, this function makes the modal visible and starts playing the video.
+
+3. **Close Modal Functionality**
+   ```js
+   span.onclick = function() {
+       modal.style.display = "none";
+       video.pause();
+       video.currentTime = 0;
+   }
+   ```
+   When the close icon (×) is clicked, the modal hides, the video pauses, and the playback resets.
+
+4. **Outside Click Close**
+   ```js
+   window.onclick = function(event) {
+       if (event.target == modal) {
+           modal.style.display = "none";
+           video.pause();
+           video.currentTime = 0;
+       }
+   }


The documentation shows outdated code using window.onclick and style.display, but the actual implementation uses modal.addEventListener and classList.remove('active').

Suggested change

btn.onclick = function() {

modal.style.display = "flex";

video.play();

}

```

When the **Play Trailer** button is clicked, this function makes the modal visible and starts playing the video.

3. **Close Modal Functionality**

```js

span.onclick = function() {

modal.style.display = "none";

video.pause();

video.currentTime = 0;

}

```

When the close icon (×) is clicked, the modal hides, the video pauses, and the playback resets.

4. **Outside Click Close**

```js

window.onclick = function(event) {

if (event.target == modal) {

modal.style.display = "none";

video.pause();

video.currentTime = 0;

}

}

btn.addEventListener("click", function() {

modal.classList.add("active");

video.play();

});

When the Play Trailer button is clicked, this function makes the modal visible and starts playing the video.

Close Modal Functionality

span.addEventListener("click", function() { modal.classList.remove("active"); video.pause(); video.currentTime = 0; });

When the close icon (×) is clicked, the modal hides, the video pauses, and the playback resets.

Outside Click Close

modal.addEventListener("click", function(event) { if (event.target === modal) { modal.classList.remove("active"); video.pause(); video.currentTime = 0; } });

sumn2u

Nice Work!

sumn2u · 2025-10-05T23:32:01Z

Fixes #326

sumn2u#326: Solution for a Video Popup Modal

8f0b211

sumn2u requested a review from Copilot October 5, 2025 10:30

Copilot AI reviewed Oct 5, 2025

View reviewed changes

FIX: Comment fixes for a Video Popup Modal

750123d

sumn2u requested a review from Copilot October 5, 2025 11:46

Copilot AI reviewed Oct 5, 2025

View reviewed changes

FIX: Update README.md and style.css for dark text color

605b250

sumn2u approved these changes Oct 5, 2025

View reviewed changes

sumn2u merged commit 93670d9 into sumn2u:main Oct 5, 2025
1 of 2 checks passed

sumn2u added the hacktoberfest-accepted label Oct 5, 2025

	- `<div class="movie-container">` — Wraps all content like the movie thumbnail, title, description, and button.
	- `<div class="movie-card">` — Wraps all content like the movie thumbnail, title, description, and button.

	- `<button id="openModal">` — Button to trigger the modal popup.
	- `<button id="openModalBtn">` — Button to trigger the modal popup.

	- `<span class="close">×</span>` — The close icon (×) used to close the modal.
	- `<button class="close-btn" id="closeModalBtn">×</button>` — The close icon (×) used to close the modal.

Conversation

soonu-kedari commented Oct 5, 2025

Uh oh!

vercel Bot commented Oct 5, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 5, 2025

Choose a reason for hiding this comment

Uh oh!

sumn2u commented Oct 5, 2025

Uh oh!

soonu-kedari commented Oct 5, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 5, 2025

Choose a reason for hiding this comment

Uh oh!

sumn2u left a comment

Choose a reason for hiding this comment

Uh oh!

sumn2u commented Oct 5, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants