refactor: gridInfoではなくnumMeasuresをinjectするようにする #2605
Conversation
|
🚀 プレビュー用ページを作成しました 🚀 更新時点でのコミットハッシュ： |
There was a problem hiding this comment.
単一の情報源から各コンポーネントが必要な情報をどこで計算するべきか、かなり悩みますね 😇 😇 😇
昔 @romot-co さんともどこかで議論した記憶。。そして同じように答えは出なかった。。
各計算結果をprovideすると、各引数に整合性が生じてStorybookでテストしづらい。（今のmainブランチ）
各コンポーネントで計算すると、複数箇所で同じ処理を書かないといけない。（このブルリク）
解決策としてはたぶんこうかな～～～と
- どちらにせよ単一の情報源からそのコンポーネントが必要な情報を計算する関数を切り出す
- 計算結果をprovideするなら
- 全計算をルートコンポーネントで行ってprovide
- Storybook内で必要最低限の関数を実行
- 👍 同じ計算を２度行わない
- 🤔 実際のコードとStorybookのコードがずれうる
- コンポーネント内で計算するなら
- 単一の情報源をprovideし、コンポーネントには必要な情報のみ引数渡しする
- 各コンポーネント内で必要最低限の関数を実行
- 👍 Storybookが簡単
- 🤔 同じ計算を行う
このどっちかなのかな～～～と思いました！！！
どっちが良いかはぶっちゃけ結論出せてないです。。。
一旦考え聞けると！！（よかった @sevenc-nanashi さんも）
| const endMeasureNumber = computed(() => numMeasures.value + 1); | ||
|
|
||
| const endTicks = computed(() => { | ||
| return measureNumberToTick( | ||
| endMeasureNumber.value, | ||
| timeSignatures.value, | ||
| tpqn.value, | ||
| ); | ||
| }); | ||
|
|
||
| const gridWidth = computed(() => { | ||
| return tickToBaseX(endTicks.value, tpqn.value) * zoomX.value; | ||
| }); | ||
|
|
||
| const gridHeight = computed(() => { | ||
| return getKeyBaseHeight() * getNumKeys() * zoomY.value; | ||
| }); |
There was a problem hiding this comment.
コードがコピペになっているのが気になりました！
https://github.com/sigprogramming/voicevox/blob/52700c33c963878c99e863f49d8d5bc1eeb0a1e4/src/components/Sing/SequencerGridSpacer.vue#L25-L41
https://github.com/sigprogramming/voicevox/blob/52700c33c963878c99e863f49d8d5bc1eeb0a1e4/src/components/Sing/SequencerGrid/Presentation.vue#L136-L153
この辺り多分整合性取れなくなりうるので、provideで与えるかどうかともかく、DRY原則に従って共通化していくのが良いかなと思いました！
ちなみにgridWidthを以前共通化を提案したときのコメントはこんな感じでした。
#2472 (comment)
| @@ -3,15 +3,42 @@ | |||
| </template> | |||
There was a problem hiding this comment.
（ただのコメントです）
このコンポーネント今思ったらなくせそうですね！！
widhtとheight指定してdiv作ってるだけなので、ScoreSequencer.vue側でこんな感じで書けば良さそう？
<div style={
width: `${gridWidth}px`,
height: `${gridHeight}px`,
"pointer-events": "none",
}/>あるいはコンポーネント化するにしても、GridSpacerではなくただのSpacerとできそうだな～と。
内容
SequencerGridはnumMeasures（小節数）のほかにgridWidthとgridHeightを受け取るようになっていますが、gridWidthはnumMeasuresに依存しているので、ストーリーブックでどちらか一方のみ変更するとSequencerGridが壊れます。なので、
numMeasuresのみ渡して、gridWidthはSequencerGrid内で算出するようにします。gridHeightもinjectするほど（共通化が必要なほど）複雑ではないので、これもSequencerGrid内で算出するようにします。（
SequencerGridSpacerの方も、SequencerGridと同様に変更します）また、
numMeasuresはシーケンサーでのみ使用されているので、domainのgetNumMeasuresとstoreのSEQUENCER_NUM_MEASURESは削除して、ScoreSequencer内で算出するようにします。（書き出し時に使用するようになるかもしれませんが、そのときに
store.getterにするのが良いかなと思います）その他