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

Add: 1~10番目のキャラクターを選択するホットキーを追加 #2034

Merged
merged 5 commits into from
May 4, 2024

Conversation

tsym77yoshi
Copy link
Contributor

内容

キャラクターのデフォルトスタイルを取得する関数を@domain/talkに移動するcommitをmerge
トークで1~10番目のキャラクターを選択するホットキーを追加

関連 Issue

close: #1187
ref: #1896

@tsym77yoshi tsym77yoshi requested a review from a team as a code owner April 28, 2024 02:39
@tsym77yoshi tsym77yoshi requested review from y-chan and removed request for a team April 28, 2024 02:39
ビルド失敗の修正
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!

ちょっとこの後コメントした内容についてのコミットをさせていただくので、 @tsym77yoshi さんから逆にレビューいただけると・・・!!
(良さそうとか、こここうすると〜みたいなコメントいただければ!)

変更は多分こちらから見れます:
523edeb?w=1

return {
action: `${index + 1}番目のキャラクターを選択` as HotkeyActionNameType,
combination: HotkeyCombination(
!isMac ? "Ctrl " + roleKey : "Meta " + roleKey,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

こうできそう!

Suggested change
!isMac ? "Ctrl " + roleKey : "Meta " + roleKey,
(!isMac ? "Ctrl " : "Meta ") + roleKey;

@@ -490,6 +495,30 @@ const removeCell = async () => {
}
};

// N番目のキャラクターを選ぶ
const selectCharacterAt = (index: number) => {
if (userOrderedCharacterInfos.value.length >= index + 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

結構長いifなので早期リターンしてあげたほうが良いかも

registerHotkeyWithCleanup({
editor: "talk",
enableInTextbox: true,
name: `${i + 1}番目のキャラクターを選択` as HotkeyActionNameType,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

番目のキャラクターを選択という文字列がasとともに使われていて、元のほうが変わったときに気付けないな〜と思いました。
共通化して番目のキャラクターを選択という文字列をconstで持っておいても良いかも・・・?

@tsym77yoshi
Copy link
Contributor Author

ありがとうございます!いいと思います!

@tsym77yoshi tsym77yoshi requested a review from a team as a code owner May 2, 2024 15:09
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

レビューありがとうございます、マージします!!

@Hiroshiba Hiroshiba merged commit 1c645f4 into VOICEVOX:main May 4, 2024
9 checks passed
@Hiroshiba
Copy link
Member

Hiroshiba commented May 4, 2024

@tsym77yoshi

非常に丁寧な進行ありがとうございました!!

結構望まれていた機能だと思うのでツイートしてみました!
https://x.com/voicevox_pj/status/1786706401240498321

他にも「次・前のキャラクターに変更」するショートカットや、スタイルを選択するショートカットなども便利だと思います!
もしよかったら実装に挑戦いただけるととても嬉しいです・・・!!!
issue作成とかだけでも・・・!!(次・前のキャラクターショートカットはissueあるかも)

@tsym77yoshi
Copy link
Contributor Author

ありがとうございます!

@Hiroshiba
Copy link
Member

Hiroshiba commented May 4, 2024

@tsym77yoshi issue作成ありがとうございます!!

上下キーの仕様は失念していました。。
こちらでいただいているプルリクエストで一旦ワークアラウンドを実装する・・・・・とかが一旦無難・・・・・?

か、keystrokeライブラリに変えるか、もろもろ頑張って自作するかですかね・・・!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

キャラクター変更ショートカットキー
2 participants