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

[WIP] 読み方&アクセント辞書ダイアログの右側パネルを別コンポーネントにする #2237

Conversation

jdkfx
Copy link
Contributor

@jdkfx jdkfx commented Aug 21, 2024

内容

  • 辞書の単語・読み、アクセントなどの追加・編集を行う部分を新しいコンポーネントに移動

関連 Issue

close #2234

スクリーンショット・動画など

その他

現状、手元で動かしている時に意図しない挙動を行っているので、以下のTODOやエラーを都度修正する

  • propsで渡すvoiceComputedの型を確認する
  • styleタグにおけるエラーを確認し、修正する

@jdkfx
Copy link
Contributor Author

jdkfx commented Aug 21, 2024

新しく切り出したコンポーネントにpropsで{ engineId: string & BRAND<"EngineId">; speakerId: string & BRAND<"SpeakerId">; styleId: number & BRAND<"StyleId">; }という型をどうやって渡してあげればいいか、ということで悩んでいます。

@Hiroshiba
Copy link
Member

engineId: EngineIdなどで可能です!
こう書いたあと、EngineIdをVSCodeでホバーすれば、EngineIdをimportする旨の提案が出てくると思います。
ファイル全体検索して見てみた感じ、たぶん・・・こう・・・? import { EngineId } from "@/type/preload";

@jdkfx
Copy link
Contributor Author

jdkfx commented Aug 21, 2024

とりあえずで新しいコンポーネントに処理を移動させて手元で動かした感じだと、編集時のインプット要素が変更されないという挙動を起こしています。
多分、propsで渡している値がreadonlyだから変更できないのかな、という可能性が考えられますかね。

@sevenc-nanashi
Copy link
Member

(まだ詳しくは見れてませんが)
DictionaryWordEditDialog側に新しい読みとかのデータを持たせても良さそう。
初期値をpropsで渡し、DictionaryWordEditDialogで編集して、emitで確定した読みとかのデータを返す、みたいな。

@jdkfx
Copy link
Contributor Author

jdkfx commented Aug 21, 2024

@sevenc-nanashi
なるほど!ありがとうございます!
その方法で実装してみます!

@jdkfx
Copy link
Contributor Author

jdkfx commented Aug 30, 2024

すみません、少しの間触れない期間が続きそうです!また触れるようになったら進捗出します!

@Hiroshiba
Copy link
Member

@jdkfx ご連絡ありがとうございます! 承知しました、またいずれぜひ・・・!!

@jdkfx
Copy link
Contributor Author

jdkfx commented Sep 22, 2024

@Hiroshiba
コンテキストメニューを呼び出す時、

  • DictionaryManageDialogコンポーネントから右側のフォーム画面であるDictionaryEditWordDialogを呼びだす。
  • DictionaryEditWordDialogのインプット要素からコンポーザブルであるuseRightClickContextMenu.tsを呼びだす。

という流れになっています。

元々あったDictionaryManageDialogから右側のフォーム画面を切り出す際、子コンポーネントに渡すpropsの値が多くなってしまい、結果的に適切な機能の切り出しができていなかったのではないかと感じています。
https://github.com/VOICEVOX/voicevox/pull/2237/files#diff-c6bf9942ebf63683aedebc287973adfe6b459ec6eb37118d4d6e926e13b72a95R184

まだWIPではありますが、設計的な点で、どの機能を切り出すのが適切かご意見をいただきたいです。

@Hiroshiba
Copy link
Member

Hiroshiba commented Sep 24, 2024

おお、なるほどです!!

流れはあってると思います!!
ただ確かに引数が結構多い気がしますね!!

多分、子コンポーネント(EditWordDialog側)だけでできるものを親コンポーネント側でやってるので、その分渡さないといけない引数が増えてるんだと思います。
ちょっとちゃんと把握できてないので間違ってたら申し訳ないのですが、wordEditingとかisWordChangedとかisOnlyHiraOrKanaとかは親コンポーネントから子コンポーネント側に移せると思います!
useRightClickContextMenuの呼び出しと同じように、setYomiとかの関数ごとうつしちゃう感じかなーと!

たぶんだけど、親コンポーネント側で持たないといけない状態変数はwordEditingだけ・・・・・・かも・・・?
uiLocked辺りが微妙かも。でもこれもたぶん子コンポーネント側だけで良さそう。

この見極め方ですが、自分の場合であれば引数で渡してるそれぞれの変数や関数の意味を捉えて、子コンポーネントだけで完結するはずのものなのか親コンポーネント側じゃないとできないのかを考え、実際移してみる感じで進める・・・かなぁ。

@jdkfx
Copy link
Contributor Author

jdkfx commented Sep 30, 2024

やり直したいので一旦クローズします。

@jdkfx jdkfx closed this Sep 30, 2024
@Hiroshiba
Copy link
Member

了解です!!
不明な点あれば気軽に聞いていただければ…!!🙏

@jdkfx jdkfx deleted the refactoring/#2234_dictionary_edit_word_dialog_component branch December 3, 2024 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants