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.

Sign up for GitHub

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

Jump to bottom

行番号を表示するオプション機能をつける #1272

Merged
merged 10 commits into from
Apr 8, 2023
Merged

行番号を表示するオプション機能をつける #1272

merged 10 commits into from
Apr 8, 2023

Conversation

wappon28dev
Copy link
Contributor

内容

オプション機能として, 行番号を追加します

関連 Issue

closes #1149

スクリーンショット・動画など

screenshot
視認性は悪くないですね...たぶん

その他

なし

@wappon28dev
Add: 行番号の追加
0bb7b3a 
- ファイルは分けたほうがいいかな...？
@wappon28dev
Copy link
Contributor Author

3桁 (width: 2rem)

0.mp4

4桁 (width: 3rem)

1.mp4

3 → 4桁 (width: 2rem → 3rem)

2.mp4

意図した挙動にはなってるはずです...

@wappon28dev wappon28dev marked this pull request as ready for review April 1, 2023 03:28
@wappon28dev wappon28dev requested a review from a team as a code owner April 1, 2023 03:28
@wappon28dev wappon28dev requested review from Hiroshiba and removed request for a team April 1, 2023 03:28
@wappon28dev
Copy link
Contributor Author

レビューの準備ができました！
この PRの差分 を見よう見まねで Settings と Store を書いたので, コードの書き方やパフォーマンスなど, 気になる箇所があればお教えください！

@wappon28dev wappon28dev changed the title WIP: 行番号を表示するオプション機能をつける 行番号を表示するオプション機能をつける Apr 1, 2023
Hiroshiba
Hiroshiba reviewed Apr 2, 2023
View reviewed changes
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です！！

image
細かいですが右揃えになってなさそうなことに気づきました･･･！

src/components/SettingDialog.vue Outdated Show resolved Hide resolved
src/store/setting.ts Outdated Show resolved Hide resolved
src/components/AudioCell.vue Outdated Show resolved Hide resolved
@wappon28dev
Copy link
Contributor Author

wappon28dev commented Apr 2, 2023 •
edited
Loading

Re: #1272 (review)
あれれ...　私の環境では右揃えになっているのですが...　変なCSSを書いてしまったですっけ...
もう一度ご確認お願いします...！

image

@sevenc-nanashi
Copy link
Member

image
image
Win・Ubuntu両方で右揃えを確認出来ました。

@Hiroshiba
Copy link
Member

右揃えの件ですが、再確認したところ大丈夫でした！！
CSSをちょっといじったりしたあとなのでhot-reloadとかで微妙な感じになっていたのかもです。お騒がせしました 🙇‍♂️

Hiroshiba
Hiroshiba reviewed Apr 4, 2023
View reviewed changes
src/store/type.ts Outdated Show resolved Hide resolved
Hiroshiba
Hiroshiba approved these changes Apr 7, 2023
View reviewed changes
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！！

設定変数の名前の変更はどちらでも大丈夫だと思います。
（個人的にはTextLineNumberとかのが良いかもと思いました！）

丁寧な進行、とても助かりました！！
次のプルリクエストをお待ちしています･･･！！

@wappon28dev
Copy link
Contributor Author

LineNumber から TextLineNumber に変更しましたので, 再度 Review お願いしますー

こちらこそありがとうございました！

sevenc-nanashi
sevenc-nanashi approved these changes Apr 8, 2023
View reviewed changes
Copy link
Member

@sevenc-nanashi sevenc-nanashi left a comment

Choose a reason for hiding this comment

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

LGTM！

@Hiroshiba
Copy link
Member

マージします！

@Hiroshiba Hiroshiba merged commit 078f5c3 into VOICEVOX:main Apr 8, 2023
@wappon28dev wappon28dev deleted the add/audio-cell-line-number branch April 8, 2023 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sevenc-nanashi sevenc-nanashi sevenc-nanashi approved these changes

@Hiroshiba Hiroshiba Awaiting requested review from Hiroshiba

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

行番号を表示するオプション機能をつける
3 participants
@wappon28dev @sevenc-nanashi @Hiroshiba