Conversation
leehyewon0531
requested review from
11t518s,
SaaaHoP,
chaeiJeon,
dltkdals224 and
yoon-junseo
SaaaHoP
left a comment
SaaaHoP
left a comment
There was a problem hiding this comment.
README로 깔끔하게 정리해주셔서 좋은거 같아요. 수고하셨습니다.
| } | ||
| ``` | ||
| ```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를 사용하지 않아도 괜찮지 않을까요...? 제 개인적인 생각이여서 다같이 고민해보면 좋을거 같아요.
| $playButton.classList.replace('fa-play', 'fa-pause') | ||
| return | ||
| } | ||
| if($video.played) { |
leehyewon0531
force-pushed
the
Hyewon/3-CustomVideoPlayer
branch
from
77c1e34 to
a7f9635
Compare
3 participants
No description provided.