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

enhance(client): Renote した時の表示をリップルエフェクトと toast に #10116

Merged
merged 6 commits into from
Mar 1, 2023

Conversation

Khsmty
Copy link
Contributor

@Khsmty Khsmty commented Feb 26, 2023

#10108 (comment)

apiWithDialog からリップルエフェクトと toast に変更しました。

image

image

@github-actions github-actions bot added the packages/frontend Client side specific issue/PR label Feb 26, 2023
@codecov
Copy link

codecov bot commented Feb 26, 2023

Codecov Report

Merging #10116 (d53ee16) into develop (81e6a21) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head d53ee16 differs from pull request most recent head 6e06d0c. Consider uploading reports for the commit 6e06d0c to get more accurate results

@@             Coverage Diff             @@
##           develop   #10116      +/-   ##
===========================================
- Coverage    25.06%   25.05%   -0.01%     
===========================================
  Files          705      705              
  Lines        65151    65167      +16     
  Branches      2307     2307              
===========================================
  Hits         16329    16329              
- Misses       48822    48838      +16     
Impacted Files Coverage Δ
...ackages/backend/src/server/api/ApiServerService.ts 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tamaina
Copy link
Contributor

tamaina commented Feb 26, 2023

catchした時の挙動どうしよう

@tamaina
Copy link
Contributor

tamaina commented Feb 26, 2023

catchなんてしない気がするからいいか

@Khsmty
Copy link
Contributor Author

Khsmty commented Feb 26, 2023

そうですねー、catch しようと思うと結構大変な気がしますねこれ

@syuilo
Copy link
Member

syuilo commented Feb 26, 2023

サーバーの応答が遅い時にレスポンスが返ってくるまで何も表示されないことになるから解決にならなそう

@syuilo
Copy link
Member

syuilo commented Feb 26, 2023

いやそうでもないか

@Khsmty
Copy link
Contributor Author

Khsmty commented Feb 26, 2023

一応右上でグルグルするので読み込み中であることは分かると思います

@tamaina
Copy link
Contributor

tamaina commented Feb 26, 2023

サーバーの応答が遅い場合でもRenoteが完了しているっぽい場合は多いし、むしろその間操作ができないのでとてもしんどい

@Khsmty
Copy link
Contributor Author

Khsmty commented Feb 26, 2023

ちなみに試しにこんな感じで rippleeffect の方も実装してみたんですが、皆さん的にはどっちの方がいいですか?
個人的には toast よりも視覚的には分かりやすいと思うんですが

image

@tamaina
Copy link
Contributor

tamaina commented Feb 26, 2023

p1.a9z.devに適用中

@tamaina
Copy link
Contributor

tamaina commented Feb 26, 2023

うーん、個人的にはRenoteできたかどうかを気にしていない(できるものだと思っている)ので邪魔なダイアログが消えてくれればどうでもいいという感想

@tamaina
Copy link
Contributor

tamaina commented Feb 26, 2023

https://p1.a9z.dev/notes/9bp47s343h

アンケート実施中

@tamaina
Copy link
Contributor

tamaina commented Feb 26, 2023

(15票しかないけど)リップルエフェクト派が優勢っぽい

@Khsmty Khsmty changed the title enhance(client): Renote した時の表示を toast に enhance(client): Renote した時の表示をリップルエフェクトに Feb 26, 2023
@tamaina tamaina requested a review from syuilo February 27, 2023 09:00
@syuilo
Copy link
Member

syuilo commented Feb 27, 2023

toastも欲しいかも

@Khsmty
Copy link
Contributor Author

Khsmty commented Feb 27, 2023

toast 追加しましたー

@Khsmty Khsmty changed the title enhance(client): Renote した時の表示をリップルエフェクトに enhance(client): Renote した時の表示をリップルエフェクトと toast に Feb 27, 2023
@tamaina
Copy link
Contributor

tamaina commented Feb 27, 2023

p1.a9z.devに適用

@tamaina
Copy link
Contributor

tamaina commented Feb 27, 2023

toast邪魔だわ

@tamaina
Copy link
Contributor

tamaina commented Feb 27, 2023

(stickyTopでいい感じにできないかしら

@Khsmty
Copy link
Contributor Author

Khsmty commented Feb 27, 2023

(stickyTopでいい感じにできないかしら

いい感じとはどんな感じでしょうか…
もう top に張り付いてはいると思うのですが

@tamaina
Copy link
Contributor

tamaina commented Feb 27, 2023

Misskeyアプリ内では、CSS変数の--stickyTopやVueでinject('CURRENT_STICKY_TOP')をtopに適用することで、ヘッダーの下部にsticky topやfixed topを指定することができる

@syuilo
Copy link
Member

syuilo commented Feb 27, 2023

トースト通知はDOM的にも概念的にも最上位に位置するからstickyTopは違うと思う

@tamaina
Copy link
Contributor

tamaina commented Feb 27, 2023

実装がどうとかは置いておいて、モバイル表示でタブにかかるのが微妙だったのでそう言った

@Khsmty
Copy link
Contributor Author

Khsmty commented Feb 27, 2023

header のサイズに合わせて 50px 下げてみました

@tamaina
Copy link
Contributor

tamaina commented Feb 27, 2023

そんなんでええんかと思ったけど割としっくりきた

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/frontend Client side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants