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

[project-s] メニューを統一して切り替えUIをメニューに追加 #1748

Conversation

Hiroshiba
Copy link
Member

内容

  • メニューをトークとソングで統一
    • 「ファイル」のとこだけはそれぞれDIできる形に
      • その過程でメニューを一部computedにしないといけなかった
  • 切り替えUIをメニュー内に移動
  • /home/talkに変更

関連 Issue

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

image

その他

これで一応mainブランチにマージしてもいいはず!!

@Hiroshiba Hiroshiba requested a review from a team as a code owner January 23, 2024 18:19
@Hiroshiba Hiroshiba requested review from y-chan and removed request for a team January 23, 2024 18:19
Comment on lines -539 to -543
// engineInfos、engineManifests、enableMultiEngineを見て動的に更新できるようにする
// FIXME: computedにする
watch([engineInfos, engineManifests, enableMultiEngine], updateEngines, {
immediate: true,
});
Copy link
Member Author

Choose a reason for hiding this comment

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

マルチエンジンの時の要リファクタリングだった項目が今回牙を剥きました 😇
もしよかったらこの辺りちょっと見ていただけると・・・・!! @sevenc-nanashi

Copy link
Member Author

@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.

移動が大半なのですが、見づらい変更になってしまっても申し訳ないです。。
もしかしたらちょっとどこか問題があるかもしれないのですが、ちょっと時間置いてからマージさせていただこうと思います・・・ 🙇

Copy link
Member

@y-chan y-chan left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@y-chan y-chan left a comment

Choose a reason for hiding this comment

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

Approveした後ですが一点だけコメント書きました...!

:deep(.q-btn[aria-pressed="true"]) {
span {
font-weight: 700;
color: colors.$display !important;
Copy link
Member

@y-chan y-chan Jan 24, 2024

Choose a reason for hiding this comment

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

Suggested change
color: colors.$display !important;
color: colors.$toolbar-button-display !important;

こちらの方が、ダークテーマにした際の視認性が向上しそうでした...!

image

Copy link
Member Author

Choose a reason for hiding this comment

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

ダークモードのこと完全に忘れてました、ありがとうございます!!
(display-on-primary`が意図としてあってそうだったのdeこっちにしようと思います!)

Copy link
Member Author

@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 cd41c30 into VOICEVOX:project-s Jan 24, 2024
7 checks passed
@Hiroshiba Hiroshiba deleted the メニューバーに切り替えを移動 branch January 24, 2024 16:29
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