-
Notifications
You must be signed in to change notification settings - Fork 310
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
feat: コンテキストメニューにアクセント句の削除を追加 #1554
feat: コンテキストメニューにアクセント句の削除を追加 #1554
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
プルリクエストありがとうございます!!
まず消去機能、素晴らしいと思います!!
レビューありがとうございました。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!!
実際に動かしてみてちゃんと動作することを確認しました!
ちょっとUX的に気になったことを列挙してみました。
- 何が消えるのかわからないかも
- 「アクセント区間を削除」にしたり、「アクセント区間」という題をおけば良さそう
- スライダーを右クリックすると値が動いちゃう
- 右クリックするとホバー中の薄い青背景が消えるのでどのアクセント区間が対象かわからなくなる
commit("COMMAND_CHANGE_SINGLE_ACCENT_PHRASE", { | ||
audioKey, | ||
accentPhrases: newAccentPhrases, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
COMMAND_CHANGE_SINGLE_ACCENT_PHRASE
、やってることはsetAccentPhrases
だけど、名称と合ってないんですよね・・・。
いつか直してあげたいですね・・・。
問題ないと思うのでマージします! |
内容
#1542 を実装しようと思いましたが、そもそもコンテキストメニューがないのでいきなり全部実装は少しハードルが高そうに見えました。
ということで、とりあえず簡単に実装できそうな「削除」機能を実装して、後から「挿入」機能を付け足そうかなと思いました。
関連 Issue
上記のための一部です。
実装中に見つけたバグです(未解決)。
スクリーンショット・動画など
その他