-
Notifications
You must be signed in to change notification settings - Fork 309
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
ツールバーのボタンをカスタマイズ可能にする #489
Changes from all commits
1da38f2
fd3c3d9
62a4dae
3cb9f21
bcd9dc5
0ba5a0e
48af645
2a7f87d
08e0223
5588bac
131aec8
73fcd3c
9886dd0
0ceb4ca
ddf4b1b
0c476e9
ba80040
1aec854
ca23ae0
3381622
34b80bb
bda7055
80fef2c
be52685
63c79ce
b5ceafd
9a85b4a
611bd8e
37b2a4a
359bde4
4caaebc
e864b62
6d23b4b
b23dba8
5e32949
6da6db0
8a6b331
b13b4b8
f7fde2e
54b5201
35ad793
143698e
0fe684b
4c47e6d
06be6e1
b34801c
93f3fb4
abddcea
fcc25d4
6102d78
cdd885a
b2ad870
6961461
45457c9
c060970
d8c8562
94d5fc9
0788ec3
972b305
b4765ca
c25232e
fac732f
d68e8c9
c390e5b
023414f
19315ca
0ac8477
b6a483b
1e144f2
702bd7d
8bb921e
4fc2b13
fdd2813
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,54 +1,44 @@ | ||
<template> | ||
<q-header class="q-py-sm"> | ||
<q-toolbar> | ||
<q-btn | ||
unelevated | ||
color="background-light" | ||
text-color="display-dark" | ||
class="text-no-wrap text-bold q-mr-sm" | ||
:disable="uiLocked" | ||
@click="playContinuously" | ||
>連続再生</q-btn | ||
> | ||
<q-btn | ||
unelevated | ||
color="background-light" | ||
text-color="display-dark" | ||
class="text-no-wrap text-bold q-mr-sm" | ||
:disable="!nowPlayingContinuously" | ||
@click="stopContinuously" | ||
>停止</q-btn | ||
> | ||
|
||
<q-space /> | ||
<q-btn | ||
unelevated | ||
color="background-light" | ||
text-color="display-dark" | ||
class="text-no-wrap text-bold q-mr-sm" | ||
:disable="!canUndo || uiLocked" | ||
@click="undo" | ||
>元に戻す</q-btn | ||
> | ||
<q-btn | ||
unelevated | ||
color="background-light" | ||
text-color="display-dark" | ||
class="text-no-wrap text-bold q-mr-sm" | ||
:disable="!canRedo || uiLocked" | ||
@click="redo" | ||
>やり直す</q-btn | ||
> | ||
<template v-for="button in headerButtons" :key="button.text"> | ||
<q-space v-if="button.text === null" /> | ||
<q-btn | ||
v-else | ||
unelevated | ||
color="background-light" | ||
text-color="display-dark" | ||
class="text-no-wrap text-bold q-mr-sm" | ||
:disable="button.disable.value" | ||
@click="button.click" | ||
>{{ button.text }}</q-btn | ||
> | ||
</template> | ||
</q-toolbar> | ||
</q-header> | ||
</template> | ||
|
||
<script lang="ts"> | ||
import { defineComponent, computed } from "vue"; | ||
import { defineComponent, computed, ComputedRef } from "vue"; | ||
import { useStore } from "@/store"; | ||
import { useQuasar } from "quasar"; | ||
import { setHotkeyFunctions } from "@/store/setting"; | ||
import { HotkeyAction, HotkeyReturnType } from "@/type/preload"; | ||
import { | ||
HotkeyAction, | ||
HotkeyReturnType, | ||
ToolbarButtonsType, | ||
} from "@/type/preload"; | ||
import SaveAllResultDialog from "@/components/SaveAllResultDialog.vue"; | ||
|
||
type ButtonContent = | ||
| { | ||
text: ToolbarButtonsType; | ||
click(): void; | ||
disable: ComputedRef<boolean>; | ||
} | ||
| { | ||
text: null; | ||
}; | ||
|
||
export default defineComponent({ | ||
setup() { | ||
|
@@ -61,6 +51,15 @@ export default defineComponent({ | |
const nowPlayingContinuously = computed( | ||
() => store.state.nowPlayingContinuously | ||
); | ||
const activeAudioKey = computed(() => store.getters.ACTIVE_AUDIO_KEY); | ||
const nowPlaying = computed(() => | ||
activeAudioKey.value | ||
? store.state.audioStates[activeAudioKey.value]?.nowPlaying | ||
: false | ||
); | ||
const toolbarSetting = computed(() => store.state.toolbarSetting); | ||
|
||
let continuouslyFlag = true; | ||
|
||
const undoRedoHotkeyMap = new Map<HotkeyAction, () => HotkeyReturnType>([ | ||
// undo | ||
|
@@ -93,7 +92,7 @@ export default defineComponent({ | |
() => { | ||
if (!uiLocked.value) { | ||
if (nowPlayingContinuously.value) { | ||
stopContinuously(); | ||
stop(); | ||
} else { | ||
playContinuously(); | ||
} | ||
|
@@ -110,9 +109,17 @@ export default defineComponent({ | |
const redo = () => { | ||
store.dispatch("REDO"); | ||
}; | ||
const playContinuously = async () => { | ||
const playAudio = async () => { | ||
try { | ||
await store.dispatch("PLAY_CONTINUOUSLY_AUDIO"); | ||
if (continuouslyFlag) { | ||
await store.dispatch("PLAY_CONTINUOUSLY_AUDIO"); | ||
} else if (activeAudioKey.value !== undefined) { | ||
await store.dispatch("PLAY_AUDIO", { | ||
audioKey: activeAudioKey.value, | ||
}); | ||
} else { | ||
throw Error(); | ||
} | ||
} catch { | ||
$q.dialog({ | ||
title: "再生に失敗しました", | ||
|
@@ -125,19 +132,100 @@ export default defineComponent({ | |
}); | ||
} | ||
}; | ||
const stopContinuously = () => { | ||
store.dispatch("STOP_CONTINUOUSLY_AUDIO"); | ||
const playContinuously = async () => { | ||
continuouslyFlag = true; | ||
await playAudio(); | ||
}; | ||
const play = async () => { | ||
continuouslyFlag = false; | ||
await playAudio(); | ||
}; | ||
const stop = () => { | ||
if (continuouslyFlag) { | ||
store.dispatch("STOP_CONTINUOUSLY_AUDIO"); | ||
} else if (activeAudioKey.value !== undefined) { | ||
store.dispatch("STOP_AUDIO", { audioKey: activeAudioKey.value }); | ||
} | ||
}; | ||
Segu-g marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const generateAndSaveAllAudio = async () => { | ||
await store.dispatch("GENERATE_AND_SAVE_ALL_AUDIO_WITH_DIALOG", { | ||
encoding: store.state.savingSetting.fileEncoding, | ||
$q, | ||
saveAllResultDialog: SaveAllResultDialog, | ||
}); | ||
}; | ||
const generateAndSaveOneAudio = async () => { | ||
await store.dispatch("GENERATE_AND_SAVE_AUDIO_WITH_DIALOG", { | ||
audioKey: activeAudioKey.value as string, | ||
$q, | ||
encoding: store.state.savingSetting.fileEncoding, | ||
}); | ||
}; | ||
const importTextFile = () => { | ||
store.dispatch("COMMAND_IMPORT_FROM_FILE", {}); | ||
}; | ||
|
||
const usableButtons: ButtonContent[] = [ | ||
{ | ||
text: "連続再生", | ||
click: playContinuously, | ||
disable: uiLocked, | ||
}, | ||
{ | ||
text: "再生", | ||
click: play, | ||
disable: computed(() => !activeAudioKey.value || uiLocked.value), | ||
}, | ||
{ | ||
text: "停止", | ||
click: stop, | ||
disable: computed( | ||
() => !nowPlayingContinuously.value && !nowPlaying.value | ||
), | ||
}, | ||
{ | ||
text: "音声書き出し", | ||
click: generateAndSaveAllAudio, | ||
disable: uiLocked, | ||
}, | ||
{ | ||
text: "一つだけ書き出し", | ||
click: generateAndSaveOneAudio, | ||
disable: computed(() => !activeAudioKey.value || uiLocked.value), | ||
}, | ||
{ | ||
text: "元に戻す", | ||
click: undo, | ||
disable: computed(() => !canUndo.value || uiLocked.value), | ||
}, | ||
{ | ||
text: "やり直す", | ||
click: redo, | ||
disable: computed(() => !canRedo.value || uiLocked.value), | ||
}, | ||
{ | ||
text: "テキスト読み込み", | ||
click: importTextFile, | ||
disable: uiLocked, | ||
}, | ||
]; | ||
Comment on lines
+168
to
+211
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. textもcomputedにすれば,連続再生ボタンを連続再生停止の為にも使えます. また,ツールバーに配置可能なactionの多くはHotKeyでも操作可能な処理が多いと思うので,その辺りと処理を纏めてactionを作ったり,disableをgetterで返したりするstoreを作ってしまってもいいかもしれません. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
確かに、これを機に再生と停止を同じボタンにまとめてしまうのは良いと思いました。 また、ボタンカスタマイズの時点で、再生と停止の両方を行えるボタンであることを示すことが難しいような気もしています。
actionにまとめてしまう感じのことは、 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
UIについて私は詳しくないので素人なのですが,押してみれば分かりますしこれまで使っていた人が最初見た時に混乱する程度でしょうか.特に記載を行わなくても問題はなさそうな気もしますが@Hiroshiba さんに意見を貰いたいです. それとは別に今気づいたのですが,ContinuslyフラグをAudioStateに置くべきだと思いました.どちらにしても現在の実装ではShift+Space等の別経路で連続再生したときに,連続再生したか単体再生したかHeaderBarでは判別ができないからです.
ショートカットキーは一部を除いてコンポーネントに所属しないグローバルな操作なのでショートカットキーの多くはactionとして記載でき,ツールバーにも置けるかなと思いましたがこのPRでする必要は特にないと思いました.testablityが改善しそうだったので提案しましたが忙しいならこのままでも構わないと思います. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 再生と停止を同じボタンにするのは別機能なので、とりあえずこのPRではしない方針で良いと思います。
こちら目からウロコでした。見逃していました・・・レビュー感謝です。 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
確かに、プレビュー時点で押せば表記が切り替わるみたいな挙動をさせるのは良いと思いました。 ただ、やはり再生(連続再生)と停止のボタン統合は別PRで行うべきな気がします。
おっと、この問題は気づきませんでした...(私がボイボにおいてほとんどホットキーを使わないので、そこまで頭が回っていませんでした...)
(もうSTOP_CONTINUOUSLY_AUDIOをSTOP_AUDIOと統合してもいいんじゃないかなって個人的には思います...) |
||
|
||
const searchButton = (button: ToolbarButtonsType): ButtonContent => { | ||
if (button === "空白") { | ||
return { | ||
text: null, | ||
}; | ||
} else { | ||
return usableButtons.find((b) => b.text === button) as ButtonContent; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 型レベルで強制できる、確かに仰る通りです。 findの使い過ぎは私も少し感じていたので、オブジェクト化を試みようと思います。 |
||
} | ||
}; | ||
|
||
const headerButtons = computed(() => | ||
toolbarSetting.value.map(searchButton) | ||
); | ||
|
||
return { | ||
uiLocked, | ||
canUndo, | ||
canRedo, | ||
nowPlayingContinuously, | ||
undo, | ||
redo, | ||
playContinuously, | ||
stopContinuously, | ||
headerButtons, | ||
}; | ||
}, | ||
}); | ||
|
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.
ToolButtonsType
を表示するテキストそのものにしているため,テキストだけを変更したいときにも状態のマイグレーションが必要になってしまいます.(表示の為の要素がstateの中に混在してしまっています).代わりに,そのボタンの機能ごとにタグを付けて管理してはどうでしょうか?(識別子とテキストの分離)
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.
なるほど、確かに言われてみれば、テキストだけを変更したい時を考えていませんでした。
識別子化は非常に良い解決策だと思うので、対応に当たりたいと思います。