Skip to content

Fix/change UI setting qall #4192

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

Merged
merged 24 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
084ea2b
設定画面(通話)の項目の順番変更、文章の一部修正、ラジオボタンのレイアウト変更
damin3A3 Dec 9, 2023
00a4c44
設定画面(通話)のレイアウト、見た目微調整
damin3A3 Dec 9, 2023
cd8048e
設定画面(通話)のレイアウト変更
damin3A3 Dec 9, 2023
267ee07
文字関連の変更を一度消した
damin3A3 Dec 9, 2023
77a781f
変更を戻した
damin3A3 Dec 9, 2023
0a6646c
コードの細かい修正
damin3A3 Dec 9, 2023
fce2b7c
Merge branch 'master' into fix/change_UI_Setting_Qall
damin3A3 Dec 9, 2023
424b9c6
Merge branch 'master' into fix/change_UI_Setting_Qall
damin3A3 Dec 28, 2023
b42c4f0
指摘をいただいたところの修正
damin3A3 Dec 28, 2023
6227ea4
prettierでのフォーマット修正
damin3A3 Dec 28, 2023
fd7e59d
タグの修正
damin3A3 Dec 29, 2023
2d0d30a
行間調整
damin3A3 Dec 30, 2023
4f77a77
コードのフォーマットの修正
damin3A3 Dec 30, 2023
56f7f20
ラジオボタンの変更を元に戻した
damin3A3 Dec 30, 2023
394a67b
スライダの背景色を消去した
damin3A3 Jan 12, 2024
1846ee5
Merge branch 'master' into fix/change_UI_Setting_Qall
damin3A3 Jan 12, 2024
3f0aa29
ラベルの色の修正
damin3A3 Jan 22, 2024
70cf5ff
vue-slider-componentを使用した(色未修正)
damin3A3 Jan 23, 2024
b473e77
フォーマット修正
damin3A3 Jan 23, 2024
2fa1e4a
データ型の統一
damin3A3 Jan 23, 2024
3c61a3a
フォーマット再修正
damin3A3 Jan 23, 2024
5c9b05c
データ型の修正を試みた
damin3A3 Jan 23, 2024
4689e71
スライダの色を調整した
damin3A3 Jan 27, 2024
1942bf9
スライダーのクリック範囲を広くした
damin3A3 Jan 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/components/Settings/QallTab/NoiseSuppression.vue
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<template>
<div>
<h3 :class="$style.header">ノイズ抑制</h3>
<div :class="$style.content">
<div>
<template v-if="isAudioWorkletSupported">
<form-radio
v-model="noiseSuppressionValue"
Expand Down Expand Up @@ -50,10 +50,9 @@ const isAudioWorkletSupported = checkAudioWorkletSupport()
.header {
margin-bottom: 8px;
}
.content {
margin-left: 12px;
}
.input {
display: block;
font-weight: bold;
Comment on lines +52 to +53
Copy link
Contributor

Choose a reason for hiding this comment

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

NatsukiくんのPRでFormRadioコンポーネント自体を変えてもらってるので、ここらへんはもしかしたらまたちょっと変えてもらうかもです

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ありがとうございます!
一旦変えずに今後の指示を待ちます。

margin-right: 8px;
}
</style>
117 changes: 57 additions & 60 deletions src/views/Settings/QallTab.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2,76 +2,35 @@
<section>
<div :class="$style.element">
<div :class="$style.enable">
<h3 :class="$style.header">RTC機能を有効にする</h3>
<div :class="$style.header_and_content">
Copy link
Contributor

Choose a reason for hiding this comment

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

imo
多分ここはh3使ってることもあってdivじゃなくてsectionタグで囲う方がいいので、header_and_contentのclassつけてるところはdivじゃなくてsectionにして、class名もsectionにするといいと思います

Copy link
Contributor Author

@damin3A3 damin3A3 Dec 28, 2023

Choose a reason for hiding this comment

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

アドバイスをありがとうございます。
header_and_contentの名前をsectionに変更して、これを使うところはdivでなくsectionタグを使うようにしました。
変更を加えなかった他の部分の見出しは元からdivですが、そこはあまり気にしなくて良いでしょうか?

Copy link
Contributor

Choose a reason for hiding this comment

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

他のところもできれば変えておいてほしいです!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ありがとうございます。わかりました。

<h3 :class="$style.header">RTC機能</h3>
Copy link
Contributor

Choose a reason for hiding this comment

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

nits
元からこうだったので、できればでいいのですが、headerじゃなくてheadingが正しいと思うので余裕あったら直してほしいです(他のh3も同様)

Copy link
Contributor Author

@damin3A3 damin3A3 Dec 28, 2023

Choose a reason for hiding this comment

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

ご指摘ありがとうございます。
修正しました。

<p>
通話などのRTC(リアルタイムコミュニケーション)機能を有効化します。
マイクなどへのアクセス許可が必要です。
</p>
</div>
<a-toggle v-model="state.isEnabled" :class="$style.toggle" />
</div>
<p :class="$style.content">
通話などのRTC(リアルタイムコミュニケーション)機能を有効化します<br />
マイクなどへのアクセス許可が必要です
</p>
</div>
<template v-if="state.isEnabled">
<div :class="$style.element">
<h3 :class="$style.header">マスターボリューム</h3>
<form-range-with-value
v-model="state.masterVolume"
:class="$style.content"
max-text="100%"
min="0"
step="0.005"
max="1"
:format="formatMasterVolume"
/>
</div>
<div :class="$style.element">
<h3 :class="$style.header">入力デバイス</h3>
<div :class="$style.content">
<form-selector
v-if="!fetchFailed && audioInputDevices.length > 0"
v-model="state.audioInputDeviceId"
:options="audioInputDeviceOptions"
/>
<p v-else>デバイスが取得できませんでした</p>
</div>
</div>
<noise-suppression
v-model="state.noiseSuppression"
:class="$style.element"
/>
<div :class="$style.element">
<h3 :class="$style.header">ノイズゲート</h3>
<form-range-with-value
v-model="state.noiseGateThreshold"
:class="$style.content"
max-text="-100dB"
min="-100"
step="1"
max="0"
:format="formatNoiseGateThreshold"
/>
<p :class="$style.content">
マイクに入力された音が指定した音量以下だった場合にミュートします<br />
-100dBにすると無効になります
</p>
</div>
<div :class="$style.element">
<div :class="$style.enable">
<h3 :class="$style.header">メッセージの読み上げ</h3>
<div :class="$style.header_and_content">
<h3 :class="$style.header">メッセージの読み上げ</h3>
<p>Qallしているチャンネルに投稿されたメッセージを読み上げます。</p>
</div>
<a-toggle v-model="state.isTtsEnabled" :class="$style.toggle" />
</div>
<p :class="$style.content">
Qallしているチャンネルに投稿されたメッセージを読み上げます
</p>
</div>
<div v-if="state.isTtsEnabled" :class="$style.element">
<h3 :class="$style.header">メッセージ読み上げの声</h3>
<div :class="$style.content">
<div>
Copy link
Contributor

Choose a reason for hiding this comment

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

nits
このdivなくてよさそうです?

Copy link
Contributor Author

@damin3A3 damin3A3 Dec 28, 2023

Choose a reason for hiding this comment

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

ご指摘ありがとうございます。
確かに要らなそうだったので、消去しました。

<form-selector
v-if="voiceOptions.length > 0"
v-model="state.voiceName"
label="読み上げボイスの種類"
:options="voiceOptions"
/>
<p v-else>読み上げ音声の声の種類が取得できませんでした</p>
<p v-else>読み上げ音声の声の種類が取得できませんでした</p>
<form-input
v-model.number="state.voicePitch"
label="ピッチ"
Copy link
Contributor

Choose a reason for hiding this comment

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

should
ラベルと上のフィールドの隙間がないのが気になるので、それぞれの間にmarginつけるようにしてほしいです
image

Copy link
Contributor Author

@damin3A3 damin3A3 Dec 28, 2023

Choose a reason for hiding this comment

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

ご指摘ありがとうございます。
cssのstyleにoptionクラスを追加し、空白をつけました。
項目(「読み上げボイスの種類」など)の文字がFigmaでは少し小さくなっているので、そこも調整しました。
また、ノイズゲートの見出しとスライダの間も隙間が少し小さく感じたので、修正しました。必要なければ元に戻します。

新規クラス

  • .option:読み上げボイスの詳細設定のクラスです。上下にmarginがあり、font-sizeを小さめにしています。
  • .content:ノイズゲートのクラスです。上下にmarginがあります。
読み上げボイスオプション_フォント調節

入力デバイスなど margin修正前
ノイズゲート_修正前

入力デバイスなど margin修正後
ノイズゲート_修正後

よろしくお願いいたします。

Expand All @@ -92,6 +51,47 @@
/>
</div>
</div>
<div :class="$style.element">
<h3 :class="$style.header">入力デバイス</h3>
<div>
<form-selector
v-if="!fetchFailed && audioInputDevices.length > 0"
v-model="state.audioInputDeviceId"
:options="audioInputDeviceOptions"
/>
<p v-else>デバイスが取得できませんでした。</p>
</div>
</div>
<div :class="$style.element">
<h3 :class="$style.header">マスターボリューム</h3>
<form-range-with-value
Copy link
Contributor

Choose a reason for hiding this comment

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

スライダー、僕のwindows+chromeの環境だとこんな感じなのですが、そちらではどんな表示ですか?(ブラウザも教えてほしいです)
image

Copy link
Contributor Author

@damin3A3 damin3A3 Dec 28, 2023

Choose a reason for hiding this comment

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

ご確認ありがとうございます。
自分の環境だとこんな感じです。

mac+chrome モバイル表示でも大体同じ感じです。
マスターボリューム_mac+chrome

mac+safari
マスターボリューム_mac+safari

iphone+iOS PWA(Safari)
確認できなかったのですが、変更なし(開発環境でないtraQ)の場合こんな感じで、今回の変更で色などは変更していないので、これと同じ感じだと思います。
マスターボリューム_iphone+PWA

Figma上のものはchromeを想定してデザインしてくださっているのかなと思うのですが、PCやブラウザに関わらず同じデザインになるようにした方がいいのでしょうか。
よろしくお願いいたします。

Copy link
Contributor

Choose a reason for hiding this comment

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

なるほどです
ブラウザによって結構変わっちゃうのでこのままでもいい気がしますが、一応周りの灰色部分消すことができるかどうかくらいでいいので、調べてみてできそうだったらやってみてほしいです!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ありがとうございます。わかりました。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

お疲れ様です。スライダーなのですが、FormRangeWithValue.vueを変更してしまってもいいですか?

Copy link
Contributor

Choose a reason for hiding this comment

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

リポジトリ全体に対して<form-range-with-valueで検索をかけてみると、このコンポーネントが今のところQallの設定画面でしか使われていないことが分かるので、OKです

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ありがとうございます。確かにQallの設定画面以外では文字列がヒットしませんでした。
では、スライダーの設定のコードを変更してみます。

v-model="state.masterVolume"
max-text="100%"
min="0"
step="0.005"
max="1"
:format="formatMasterVolume"
/>
</div>
<div :class="$style.element">
<h3 :class="$style.header">ノイズゲート</h3>
<p>
マイクに入力された音が指定した音量以下だった場合にミュートします。
-100dBにすると無効になります。
</p>
<form-range-with-value
v-model="state.noiseGateThreshold"
max-text="-100dB"
min="-100"
step="1"
max="0"
:format="formatNoiseGateThreshold"
/>
</div>
<noise-suppression
v-model="state.noiseSuppression"
:class="$style.element"
/>
</template>
</section>
</template>
Expand Down Expand Up @@ -211,15 +211,12 @@ const voiceOptions = useVoices()
.element {
margin: 24px 0;
}
.content {
margin-left: 12px;
}
.enable {
display: flex;
align-items: center;
margin-bottom: 8px;
h3 {
margin: 0;
.header_and_content {
Copy link
Contributor

Choose a reason for hiding this comment

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

nits
class名はcamelCaseにしてほしいです:pray:

Copy link
Contributor Author

@damin3A3 damin3A3 Dec 28, 2023

Choose a reason for hiding this comment

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

ご指摘ありがとうございます。
下でアドバイスをいただいている通り、class名をsectionにしました。

flex: 1;
}
.toggle {
margin-left: 12px;
Expand Down