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

キャラクター選択ボタンを押した時に一瞬重くなる問題を解決したい #1627

Open
Hiroshiba opened this issue Oct 28, 2023 · 7 comments

Comments

@Hiroshiba
Copy link
Member

不具合の内容

テキスト欄の左側にキャラクターアイコンが表示されていて、それをクリックするとキャラクター一覧が現れると思います。
このUIが実は結構重いので解決したいです。

色々調べた感想ですが、おそらくコンポーネントかDOMが大量生成されているのが原因だと思います。
大量のコンポーネントが作られる理由は、キャラクターリストを表示するquasasrのq-menuがVueのteleportで実装されており、teleportはコンポーネントをキャッシュ?せずに毎回再生成するから・・・だと思っています。

現象・ログ

キャラクター選択ボタンを押すと、PCによっては開くまで結構時間がかかる。

再現手順

キャラクター選択ボタンを押す。
開発環境の場合はelectronのVue拡張機能を使うと凄まじ量のコンポーネントが毎回作られてることがわかる。

期待動作

少なくとも最初の1回以外はサクッと開けること。

その他

解決策としては、なんとかしてteleportでもコンポーネントのキャッシュ?が効くようにするか、はたまたquasarのq-menuに頼らない実装にするかかなと思っています。
Vueにあまり詳しくないですが、前者はどうしようもない気がしています。
後者は気合でコンポーネントを作って、ボタンの下にコンポーネントが開かれるように自作UIを作れば良いような気がしてますが、それはそれでハミ出ないようにするなどの機構が大変かも。

キャラクターが増えれば増えるほど重くなっていくので、おそらく重要度は高いと見込んでおり、優先度を中にしています。
VueやDOMに詳しい方がいらっしゃったら是非解決に挑戦してみて欲しいです・・・。

@tunamaguro
Copy link
Contributor

tunamaguro commented Apr 13, 2024

こんにちは!

こちらについてメニューをキャッシュする方針で簡単に実装してみました。Hiroshibaさんのイメージしていたのはこのような雰囲気のものでしたでしょうか?

menu-sample

やっていること自体は1度だけコンポーネントをマウントして、そのあと可視性を切り替えているだけです

@Hiroshiba
Copy link
Member Author

@tunamaguro コメントありがとうございます!

実装わからないのでもしかしたら違うかもなのですが、たぶんまさしくな気がしました!!!
この状態で、追加で1つめのテキスト欄のキャラ選択ボタンも初回爆速でかつクリックした付近に展開されるなら、まさしくだと思います!!!

@tunamaguro
Copy link
Contributor

@tunamaguro コメントありがとうございます!

実装わからないのでもしかしたら違うかもなのですが、たぶんまさしくな気がしました!!! この状態で、追加で1つめのテキスト欄のキャラ選択ボタンも初回爆速でかつクリックした付近に展開されるなら、まさしくだと思います!!!

1つ目のキャラ選択ボタンの初回クリックは、おそらく遅いままだという予想です。私がVueに詳しくないのと、このコンポーネントの実装をしっかり読んでいないので分かりませんが、propsとして値をセットする関数を渡されていると思います。その場合キャッシュ効かないはずなので、キャラ選択ボタンごとに初回は遅く、それ以降早くという風になると思います

@tunamaguro
Copy link
Contributor

実装は家に帰り次第、ドラフトPRとしてお見せします

@tunamaguro
Copy link
Contributor

完全に忘れていて遅くなってしまいましたが、現状の実装をドラフトPRにしました(#1994)

今の段階で考えないといけない、やらないといけないと思っていることは以下のX個です

  1. そもそも2回目以降の表示を早くするという方針で良いか
    自分でやっておいてあれですが、キャラクター選択ボタンを2回以上押すシーンがあまり想像できませんでした。その場合、実装の複雑さを増すだけになってしまうかもしれません
  2. 挙動変化の対応
    quasarのMenuにはEscで閉じる機能が付いていますが、現状それがないので必要であれば実装しないといけません。今のところ気づいているのはそれくらいですが、もしかするとほかにも変わるところがあるかもです。
  3. キャラクター選択ボタンを連続で押すと重なってしまう問題への対処
    現状の実装でキャラクター選択ボタンを連続で押すと、メニューが重なってしまいます
    重なる
    これは、現状可視性の切り替えに使っている、clickイベントがボタンに吸われていることが原因だと思っています。これに関しては、メニューを開く前に都度すべてのメニューを閉じるような処理を加えることで対策できると思います

@Hiroshiba
Copy link
Member Author

@tunamaguro すみません大変遅くなりました、ドラフトPRありがとうございます!!

見させていただいたのですが、すみません!!キャッシュを作る形が思ってたのと違ってることに気づきました!
キャラクターボタンごとにキャッシュを作る形ではなく、グローバルに1回作ったものを使います形を想像していました!

頂いたプルリクエストおそらく同じボタンでキャッシュを持つ形ですよね。
おっしゃる通りこの場合嬉しい方もいらっしゃるかもですが、これを開く場合はだいたい新しいテキスト欄のキャラクターを変更する時だと思うので、大半の需要は捉えられないのかなと思いました。
言葉足らずで申し訳ないです! 🙇

実を見させていただいた感じかなりシンプルに感じました。
このTeleportされているものをコンポーネントの外から渡して使い回せる形にできれば、もしかしたら完成・・・?(うーん、できるのかな。。)

@Hiroshiba
Copy link
Member Author

あと今手元で再検証したところ、そもそも重くないのでは説が自分の中で浮上してきました。
Vueのパフォーマンスコンソールを開いてたから遅かったり、あるいは開発環境だったから遅い可能性があるかも・・・?

実際に配布されている製品版でキャラクターボタンを押した時に遅いかどうかを一応検証した方がいいかも。
(というのを Discord でお願いしてみました 🙇 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants
@Hiroshiba @tunamaguro and others