-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: add packages/karaoke #65
Merged
Merged
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
…ile entring/leaving the viewport
…manually We set `audio.currentTime` and `audio.muted` manually for the following situation: The default behavior is to autoplay the audio and quote shadow animation. However, the modern browser will block autoplay if the audio is not set to `muted`. (For more information, please see https://developer.mozilla.org/en-US/docs/Web/Media/Autoplay_guide#autoplay_availability) (aftermath: `audio.play()` will return promise rejection) And the unlucky part is that we cannot set `muted` in the `audio` tag due to this known issue (facebook/react#10389). (aftermath: we have to change `muted` attribute by manipulating DOM element) Because of the above issues, we cannot play the audio with sound only if users click the audio button. At the same time, the quote animation works well. (aftermath: audio and shadow animation are not synced) When the users click the audio button, we have to play the audio at the right `currentTime` to catch up the shadow animation. (counting the `currentTime` by ourselves) And this patch handles the above situation.
nickhsine
force-pushed
the
audio-quote-shadow
branch
from
July 12, 2022 04:09
969f37c
to
45b89d5
Compare
nickhsine
commented
Jul 12, 2022
@caesarWHLee 有空請幫忙 review 一下。 |
nickhsine
commented
Jul 12, 2022
nickhsine
commented
Jul 12, 2022
nickhsine
commented
Jul 12, 2022
See related issue: video-dev/hls.js#1686 Since there is a problem with `canplay` event, this patch removes `audioOpts.canPlay` state to avoid broken functionaility on sarafi.
nickhsine
force-pushed
the
audio-quote-shadow
branch
3 times, most recently
from
July 14, 2022 02:39
333fb9d
to
6d14ec9
Compare
… click the buttons This patch fixes a corner case. On safari, `audio.play()` still returns Promise rejection even users have already had interactions (clicked the buttons). My guess is because `audio` DOM element is regenerated due to `Karaoke` component's re-rendering. This patch moves `<audio>` tag out of `<Container>`. Therefore, even if `Karaoke` component re-renders, React won't regenerate a new `audio` DOM element.
nickhsine
force-pushed
the
audio-quote-shadow
branch
from
July 14, 2022 02:53
6d14ec9
to
3c10d01
Compare
dyfu95
reviewed
Jul 14, 2022
commit 3c10d01 only fixes one Karaoke component on the Safari browser. If there are several Karaoke components on the Safari browser, `audio.play()` still encounter Promise rejection.
nickhsine
force-pushed
the
audio-quote-shadow
branch
from
July 14, 2022 05:06
0108024
to
06eb881
Compare
這個我會再調整 intersection observer 的 threshold 試試看。 |
dyfu95
reviewed
Jul 14, 2022
nickhsine
commented
Jul 14, 2022
caesarWHLee
reviewed
Jul 14, 2022
nickhsine
added a commit
to nickhsine/readr-react
that referenced
this pull request
Jul 14, 2022
nickhsine
added a commit
to nickhsine/readr-react
that referenced
this pull request
Jul 14, 2022
…iew comments Address comment: readr-media#65 (comment)
nickhsine
added a commit
to nickhsine/readr-react
that referenced
this pull request
Jul 14, 2022
…iew comment Review comment: readr-media#65 (comment)
nickhsine
added a commit
to nickhsine/readr-react
that referenced
this pull request
Jul 14, 2022
nickhsine
commented
Jul 15, 2022
dyfu95
reviewed
Jul 15, 2022
LGTM 👍 👍 |
LGTM 💯 ⭐ ✨ |
@caesarWHLee @dyfu95 |
nickhsine
force-pushed
the
audio-quote-shadow
branch
from
July 19, 2022 08:25
db3e287
to
85c1ddd
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.
Feature 描述
根據 mockup 實作卡拉 OK 元件。
互動需求說明:
Demo:
實作細節
有兩點特殊之處需要先加以說明:
<audio>
tag 不支援muted
屬性,這是一個 known issue,相關討論請見 <video /> attribute needed but not guaranteed by React facebook/react#10389autoplay
的話,必須搭配靜音模式muted={true}
才會生效原本的實作方式是想透過
<audio autoPlay muted>
來讓瀏覽器幫忙自動播放音檔,並且記錄音檔的播放到哪裡currentTime
。但因為muted
屬性 React 不能使用,因此,放棄該實作方式,而改用較為複雜的方法。較為複雜的方法是:
currentTime
audio.play()
,暫停audio.pause()
和開啟/靜音模式audio.muted=(true|false)
currentTime
的記錄更新是透過setTimeout
實作,讓字卡一邊跑動畫,一邊更新currentTime
。另外,此實作利用 intersection observer 來判斷元件是否進入 viewport。
因為有可能有多個卡拉 OK 元件出現在同一個網頁上,所以將
muted
state 記錄在 window level;當使用者在其中一個卡拉 OK 元件中開啟靜音模式,其他的卡拉 OK 元件也都會一起開啟靜音模式。TODOs