Refactor: MenuBar.vueをApp.vueに持ってくる #1966
Conversation
There was a problem hiding this comment.
ほぼLGTMです！！
コードも移動のみが大半なことを確認しました。
びっくりするくらいきれいなコード配置になりましたね。
ダイアログを表示する関数がVuex側にあるのたぶん変なんだよな〜と思って設計を考えたのですが、メニューアイテムの挙動もHotkeyActionと同様に「ただ登録された関数を実行する」だけにすれば綺麗だと気づきました。
そもそもItemMenuにはショートカットキーを良い感じに設定できるべきですし、ちょうど良さそう。
今のHotkeyActionをより抽象度の高いなんとかActionにして、それをItemMenu辺りに渡せるようにする設計･･･？
たぶんregisterも何か別のManagerかPluginに移して、HotkeyPluginはただそれをバイパスするだけ、みたいな設計どうですかね。（このPRに関係なく）
|
反映しました。 |
There was a problem hiding this comment.
LGTM！！
とてもきれいな切り出しだったと思います。
この辺だいぶ片付いてきて気持ちよくコード書けるようになってきた感ありますね！
#1966 (review) に書いたコメントをまとめてissue作ろうかなと思ってるのですが、Action周り詳しい @sevenc-nanashi さんに最初にちょい聞いておきたいなと。
HotkeyActionをより抽象化して、メニューのアクションも同じActionを見れるようにし、ActionManagerみたいなのを作るみたいな設計かなぁと。
･･････うーーーーん。できそうだけど、若干落とし穴ありそうなんですよねぇ。
いやーーーー大丈夫かも。。。。うーん。。。。
いやregisterHotkeyWithCleanupはホットキー関係なくアクションの登録で済ませられてるから、本当にActionManagerかPluginを作って、HotkeyPluginから機能を分離し、Menuから見れるようにすれば良い･･･？
けど「辞書を開く」みたいなわりとショートカットキーがいらない寄りの機能がショートカットキーリストに出ちゃう･･･かと思ったけど、そういうのは普通にMenuに関数渡せるようにしておけば良いかも。
なんかすごいきれいに作れる気がしますね！！！！！！！
| @@ -468,4 +473,64 @@ export const uiStore = createPartialStore<UiStoreTypes>({ | |||
| dispatch("SET_PROGRESS", { progress: -1 }); | |||
| }, | |||
| }, | |||
|
|
|||
| // TODO: この4つのアクションをVue側に移動したい | |||
内容
fileSubMenuData、editSubMenuDataをcomposableにし、MenuBarをAppに持ってきます。
関連 Issue
スクリーンショット・動画など
（なし）
その他
（なし）