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

Change: 選択中のキャラを下から無くさないように #1703

Conversation

sevenc-nanashi
Copy link
Member

内容

タイトル通り。

関連 Issue

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

image

その他

separator周りで迷ってます。

@sevenc-nanashi sevenc-nanashi requested a review from a team as a code owner January 10, 2024 11:48
@sevenc-nanashi sevenc-nanashi requested review from y-chan and removed request for a team January 10, 2024 11:48
@Hiroshiba Hiroshiba requested review from Hiroshiba and removed request for y-chan January 17, 2024 18: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.

ちょっとバグ見つけました!
選択中のキャラがスタイルありだと、一番上のキャラのスタイルを展開した時になぜか一緒に展開されるかも・・・?

あとデザインをすごく悩んでます 😇
ちなみに一番上の方の選択中AudioCellだけ背景色を変えないようにするのとかって、難しそうだったりしますか・・・・?

@sevenc-nanashi
Copy link
Member Author

ちなみに一番上の方の選択中AudioCellだけ背景色を変えないようにするのとかって、難しそうだったりしますか・・・・?

割と設計が難しくなると思います。
プロジェクト内のキャラクターを表示するセクションを作るとかそう言う感じのことをするためにCharacterInfo[][]を受け取らせてる(showSelectedCharacterOnTopではなく)ので、良い感じに拡張性を持たせつつ汎用性があるようにしたい...

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.

プロジェクト内のキャラクターを表示するセクションを作るとかそう言う感じのことをするためにCharacterInfo[][]を受け取らせてる(showSelectedCharacterOnTopではなく)ので、良い感じに拡張性を持たせつつ汎用性があるようにしたい...

なるほどです、そこはかなりよくわかります。

うーーーーーん!!!決断がすごく難しいですね!
というのも将来的にこのセクションを別のコンポーネントに分けたくなった時、二重構造になったこのプルリクエストの状態から分けるのが大変になりそうなんですよね・・・。
かと言って現状デザインが確定しているわけでもないので、どうコンポーネントを分けた方がいいかも分からない・・・。

実装を足すと、ややこしいCharacterButtonコンポーネントがさらに複雑化し、あとで機能を足すのが相当大変になっちゃうと思います。
まずリファクタリングするのはどうでしょう?多分こんな感じに解体できそうかなと。

  • ChatacterButton
    • Container.vue ← Vuexと連携して↓を制御するだけ
    • CharacterButton.vue ← ボタン部分だけ
    • CharacterList.vue ← キャラクターリストを表示するだけ
    • StyleList.vue ← スタイルリストを表示するだけ

ここまで崩しておけば仮にコンポーネント分けたくなっても、CharacterList.vueをコピーして別のコンポーネント作ればいいだけなので。

ちょっとデザイン的・実装的課題がありそうで0.15に間に合わなそうです。申し訳ない。
逆に言うと時間ができるのでもしよかったらリファクタリングできるとめちゃくちゃ助かります・・・!!!

@sevenc-nanashi
Copy link
Member Author

ちょっとデザイン的・実装的課題がありそうで0.15に間に合わなそうです。

今のままで0.15出すとそれはそれで大変だと思います、2回仕様変更されるので

@sevenc-nanashi
Copy link
Member Author

一旦CharacterButtonをリファクタリングしたいのでCloseします。

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