[혜원] 3-CustomVideoPlayer #17
Open
Conversation
This file contains 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
leehyewon0531
requested review from
11t518s,
chaeiJeon,
dltkdals224,
SaaaHoP and
yoon-junseo
SaaaHoP
reviewed
Apr 2, 2022
| } | ||
| ``` | ||
| ```javascript | ||
| $video.currentTime = Math.trunc(currentPosition) |
There was a problem hiding this comment.
Math.trunc는 처음 알았네요. 소수점을 다 떼주는 역할을 하네요. Math.floor 와 같은 역할을 하는거 맞겠죠?
| - - - | ||
| - video 의 메타 데이터가 로드되면 해당 이벤트가 발생한다. | ||
| - video 의 전체 길이를 가져오는 duration 속성은, **video 에 대한 로드가 끝나지 않은 시점에서 호출하면 NaN 값이 출력된다.** | ||
| - 따라서 로드가 끝난 시점에서 duration 값을 호출하고 싶다면, **'lodaedmetadata'** 이벤트 리스너를 등록해야 한다. |
There was a problem hiding this comment.
저도 duration NaN 떴었는데 비디오 정보를 불러오기전에 접근했어서 그랬던거였어요. 굳입니다!!
| ```javascript | ||
| $video.addEventListener('loadedmetadata', function() { | ||
| const duration = $video.duration | ||
| const unit = ((parseFloat($progressBar.max) - parseFloat($progressBar.min)) / duration) / 4 |
There was a problem hiding this comment.
unit이 정확히 어떤 역할을 하는지 알기 힘든거 같아요. 어떤 역할을 하나요?
| $playButton.classList.replace('fa-play', 'fa-pause') | ||
| return | ||
| } | ||
| if($video.played) { |
There was a problem hiding this comment.
if를 두번 쓸필요 없이 위에서 early return 해줬으니 paused가 아닌경우는 if 없이 해줘도 될거 같아요 아마도...?
| if($playButton.classList.contains('fa-pause')){ | ||
| $playButton.classList.replace('fa-pause', 'fa-play') | ||
| } | ||
| $progressBar.value = 0 |
There was a problem hiding this comment.
video.curretTime = 0 해주면 value도 같이 0 되지않나요? 잘 모르겠어서...
| }) | ||
| }) | ||
| ``` | ||
| loadedmetadata 항목의 설명에서도 언급했듯이, duration 은 메타데이터가 로드된 이후에 호출하는 것이 안전하다. 그럼 duration 과 관련된 변수를 사용하는 event listener 들은 loadedmetadata 로 duration 을 가져온 이후에 실행됨이 보장되어야 할 것 같은데, 그렇게 할 만한 방법이 중첩밖에 생각나지 않았다... 이게 효율적인 방법인지는 잘 모르겠다... (nested event listener 이런 식으로 서치해 봤는데 좋은 정보를 얻지 못했다.) |
There was a problem hiding this comment.
제 생각인데 문제가 되는게 메타데이터 불러오기 전에 비디오 데이터를 접근하려해서인거 같은데, 이벤트 리스너는 결국 동영상이 재생될때 작동하고 동영상을 재생할때는 메타데이터가 다 불러와졌으니 loadedonmetadata를 사용하지 않아도 괜찮지 않을까요...? 제 개인적인 생각이여서 다같이 고민해보면 좋을거 같아요.
chaeiJeon
reviewed
Apr 3, 2022
| $playButton.classList.replace('fa-play', 'fa-pause') | ||
| return | ||
| } | ||
| if($video.played) { |
leehyewon0531
force-pushed
the
Hyewon/3-CustomVideoPlayer
branch
from
77c1e34
to
a7f9635
Compare
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.
No description provided.