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

React/radio controllers #378

Merged
merged 16 commits into from
Jul 13, 2019
Merged

Conversation

kobakazu0429
Copy link
Member

@kobakazu0429 kobakazu0429 commented Jul 9, 2019

closed #323

WHAT

やったこと


export default ({ url, duration }: IProps): IUseAudio => {
const [audio] = React.useState(new Audio());
const [, _forceUpdate] = React.useState(false);
Copy link
Member

Choose a reason for hiding this comment

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

forceUpdateが何をしているのかちょっとよく分からなかったので教えてほしいです!

Copy link
Member Author

Choose a reason for hiding this comment

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

内部状態に変更があった時に直前の真偽値とは逆の値をセットすることで必ず再レンダリングさせるためです

Copy link
Member

Choose a reason for hiding this comment

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

webAPIにあんまり詳しくないから教えてほしいんだけど、
addEventListenerremoveEventListenerの第2引数にboolを渡したらどんな挙動になるんです?

Copy link
Member Author

Choose a reason for hiding this comment

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

第2引数に渡しているのはconst forceUpdate = () => _forceUpdate(prevState => !prevState);という関数でboolは渡してないです。
この関数を渡すことでaudio.currentTimeなどの値が変わると必ず再レンダリングされます。
これがないとaudio.currentTime等が更新されないのでシークバーなども再レンダリングされません。

Copy link
Member

Choose a reason for hiding this comment

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

たしかに、boolじゃなかった、

この関数を渡すことでaudio.currentTimeなどの値が変わると必ず再レンダリングされます。
これがないとaudio.currentTime等が更新されないのでシークバーなども再レンダリングされません。

なぜこのtoggleしている?関数を渡すと再レンダリングされるようになるのかがよく分かっていない、、、

Copy link
Member

Choose a reason for hiding this comment

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

stateが変われば再レンダリングされるっていう仕様を使ってcurrentTimeとかをobserveしてるのね、なるほど
トリッキーな感じがするけどこれが普通なのかな、、

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

確かにuseCallbackの方が正しそう
別PRで修正してもいいですか?
一旦このRadioCard系を潰したいです

Copy link
Member

Choose a reason for hiding this comment

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

別PRで修正してもいいですか?

OK!

Copy link
Member Author

Choose a reason for hiding this comment

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

#380

@kobakazu0429
Copy link
Member Author

useAudioの参考URL
https://qiita.com/kazumicho/items/53572ae56bb70b2e20d0)


export default ({ url, duration }: IProps): IUseAudio => {
const [audio] = React.useState(new Audio());
const [, _forceUpdate] = React.useState(false);
Copy link
Member

Choose a reason for hiding this comment

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

あーちょっと分かってきたかもしれない!

frontend/src/utils/hooks/useAudio.ts Outdated Show resolved Hide resolved

export default ({ url, duration }: IProps): IUseAudio => {
const [audio] = React.useState(new Audio());
const [, _forceUpdate] = React.useState(false);
Copy link
Member

Choose a reason for hiding this comment

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

stateが変われば再レンダリングされるっていう仕様を使ってcurrentTimeとかをobserveしてるのね、なるほど
トリッキーな感じがするけどこれが普通なのかな、、

cx={params.width / 2}
cy={params.height / 2}
r={params.radius - 2}
/>
Copy link
Member

Choose a reason for hiding this comment

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

めちゃくちゃcircleを作っている...

Copy link
Member Author

Choose a reason for hiding this comment

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

あのデザインは難しかった

Copy link
Member

@euglena1215 euglena1215 left a comment

Choose a reason for hiding this comment

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

多分良いと思う!お疲れさま!

@kobakazu0429
Copy link
Member Author

マージしますー!

@kobakazu0429 kobakazu0429 merged commit ab8b4e0 into react/RadioCard-master Jul 13, 2019
@kobakazu0429 kobakazu0429 deleted the react/radio-controllers branch August 17, 2019 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants