Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add setSource method #153

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ The player instance exposes the following methods, accessible when the player is
| `mute()` | - | - | Mute the volume |
| `unMute()` | - | - | Unmute the volume |
| `seekTo(time)` | `Number` | - | Seek to a current time in seconds |
| `setSource(source)` | `String` | - | Set the media source |
Copy link
Member

Choose a reason for hiding this comment

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

It should be added a mention to specify that it is only available for HTML5 video (and audio?)

You can get inspiration from similar examples in the README

https://github.com/vlitejs/vlite/blob/main/README.md?plain=1#L266-L270

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should it be implemented also for other non-HTML5 providers?

Copy link
Member

Choose a reason for hiding this comment

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

I've check and other providers have method to dynamically change the video source. Good news! And I prefer have common use case between providers.

The change can finally be in Player class but the same method needs to work with all providers. You can inspired from other method for the logic. Their method seems to not be a Promise.

The commun parameter seems to be videoId which contains the new video ID.

Example of implementation:

diff --git a/README.md b/README.md
index 669135a..aa3497d 100644
--- a/README.md
+++ b/README.md
@@ -295,6 +295,7 @@ The player instance exposes the following methods, accessible when the player is
 | `getDuration()`        |         -          | `Promise` | Get the duration                  |
 | `mute()`               |         -          |     -     | Mute the volume                   |
 | `unMute()`             |         -          |     -     | Unmute the volume                 |
+| `setSource()`          |      `String`      |     -     | Set the new source from video ID  |
 | `seekTo(time)`         |      `Number`      |     -     | Seek to a current time in seconds |
 | `requestFullscreen()`  |         -          |     -     | Request the fullscreen            |
 | `exitFullscreen()`     |         -          |     -     | Exit the fullscreen               |
diff --git a/src/core/player.ts b/src/core/player.ts
index 82598ab..e715ff5 100644
--- a/src/core/player.ts
+++ b/src/core/player.ts
@@ -168,6 +168,13 @@ export default abstract class Player {
 	 */
 	public abstract methodUnMute(): void
 
+	/**
+	 * methodSetSource
+	 * Extends by the provider
+	 * @abstract
+	 */
+	public abstract methodSetSource(videoId: string): void
+
 	/**
 	 * On the player is ready
 	 */
@@ -455,6 +462,14 @@ export default abstract class Player {
 		this.dispatchEvent('volumechange')
 	}
 
+	/**
+	 * Set the new source of the player
+	 * @param videoId Video ID
+	 */
+	setSource(videoId: string) {
+		this.methodSetSource(videoId)
+	}
+
 	/**
 	 * Update the current time of the media element
 	 * @param newTime New current time of the media element
diff --git a/src/providers/vimeo/vimeo.ts b/src/providers/vimeo/vimeo.ts
index ba59476..8c2ea7d 100644
--- a/src/providers/vimeo/vimeo.ts
+++ b/src/providers/vimeo/vimeo.ts
@@ -147,6 +147,10 @@ export default function VimeoProvider(Player: any) {
 			})
 		}
 
+		methodSetSource(id: string) {
+			this.instance.loadVideo(id)
+		}
+
 		/**
 		 * Play method of the player
 		 */

Copy link
Member

Choose a reason for hiding this comment

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

videoId parameter name is okay for Youtube, Vimeo and Dailymotion but not for HTML5 which have a video url. Needs to adapt

| `requestFullscreen()` | - | - | Request the fullscreen |
| `exitFullscreen()` | - | - | Exit the fullscreen |
| `getInstance()` | - | - | Get the player instance |
Expand Down
9 changes: 9 additions & 0 deletions src/providers/html5/html5.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ export default function Html5Provider(Player: any) {
})
}

/**
* Set the media source
* @param source New media source URL
*/
setSource(source: string) {
this.media.src = source
this.media.load()
}

/**
* Create event listeners
* All listeners are created on class properties to facilitate the deletion of events
Expand Down