キー割り当て画面からホットキー設定ダイアログを抜き出してコンポーネント化する #2209
Conversation
| <SetHotkeyDialog | ||
| :isHotkeyDialogOpened | ||
| :lastAction | ||
| :lastRecord | ||
| :duplicatedHotkey |
There was a problem hiding this comment.
ちょっと相談が！
今このコンポーネントは表示専用になっていて、内部で処理を一切行なってない形になっています。
例えば「ショートカットキー入力を受け付ける」などの処理は親コンポーネントで行ってる形です。
そういった処理もこのコンポーネントの中に移動することもできそうですが、どの処理を親コンポーネントで行い、どの処理をコンポーネントで行うのが良さそうでしょうか？
結構ここのコンポーネントの切り分けは難しく、今のプルリクエストのよう表示だけを担わせるのもありだと思います。
もうちょっと踏み込んでショートカットキーの入力機構も切り分けたコンポーネントの方にあってもいいかもです。
というのどう思われるかお聞きしたいです！
もし移動した方が良さそうな場合であっても、移動を伴うプルリクエストなので、追加で移動をするのではなく一旦このプルリクエストをマージできると嬉しいです･･･！
There was a problem hiding this comment.
入力に関する処理も切り分けたコンポーネントに移し替えるのがいいのかなと考えています。
コンポーネントの責務として
- 元々あったコンポーネント: ショートカットキーの表示、検索などの表示に関わる部分
- 新しく切り出したコンポーネント: ショートカットキーの編集、入力、未設定にするなど入力に関わる部分
とすると、責務がはっきりしやすいのではないかと考えます！
ただ、Vueやフロントエンドの知識に精通しているわけではないので、もしかすると正しいことを言っていない可能性があります... 🙇
There was a problem hiding this comment.
なるほどです、認識が揃って良かったです！
「本当はこうしたいけど、今はそうなってない」というのがわかるようにコメントを追加するのはどうでしょう？
例えばrecordCombination関数の辺りに「// FIXME: HotkeyRecordingDialog内に移動する」と書いとくとか！
で、もしよかったらぜひ入力部分を切り分ける部分も（あとあと）挑戦してみていただけると･･･！！
There was a problem hiding this comment.
recordCombination 以外にも、changeHotkeySettingsも新しく作成したコンポーネントに移動できそうだと考えているため、そちらについてもコメントを追加しておきました。
| @deleteHotkey="deleteHotkey" | ||
| @changeHotkeySettings="changeHotkeySettings" |
There was a problem hiding this comment.
（ただのコメントです）
これちょっとコンポーネント分けの難しいところというか、僕の中でも結論が出てないのですが、一般的に言われていることをちょっとご紹介してみます 🙏
子コンポーネントから親コンポーネントの処理を実行したいとき、今の実装のように具体的な処理をする関数を渡す方法と、イベントのみやり取りする方法の2つがあります。
例えばイベントペースでやり取りする場合、「追加ボタンが押された」「上書きボタンが押された」というコールバックを受け取って、親コンポーネント側が具体的な処理を書く感じです。
イベントをやり取りする方が疎結合になってテストがやりやすいため、一般的にはこっちのがいいのですが、今回の場合みたく処理が複雑に絡み合ってる場合は直接関数を渡しても良いかもです。
「イベントのやり取りにできないか」という考え方に立つと何か見えてくるかもです。
まあ慣れてきたらぜひ･･･！
There was a problem hiding this comment.
ありがとうございます！！今後の実装に活かしたいです！！
内容
/components/Dialog/SetHotkeyDialog.vueを作成関連 Issue
close #2002
スクリーンショット・動画など
その他