Open ocknamo opened 3 months ago
The latest updates on your projects. Learn more about Vercel for Git ↗︎
Name | Status | Preview | Comments | Updated (UTC) |
---|---|---|---|---|
nostter | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Oct 3, 2024 4:03pm |
まだコードまではちゃんと読めていないんですが軽く触った感じ以下の点が気になりました。
@SnowCait 一旦すぐ修正できる以下について修正しました
他の指摘いただいた点についていくつか確認させてください。
これは設定画面に項目を追加するイメージでしょうか? どこに追加するなどの具体的なイメージはありますか? (自分としてはメディアプレビューの下にチェックボックスを追加するのが一番普通かなと思いました)
これはバグだと思うので調べたいのですが再現するユーザがこちらで見つけられなかったので、発生しているユーザのnpubなどわかりましたら共有いただきたいです。
よろしくおねがいします
@ocknamo
- 画像プロキシの有効/無効を選べるオプションが欲しい(トラブルがあったときにユーザー側で回避できるようにだったり、(自分には分からなかったけど)画質が気になる人だったり)
これは設定画面に項目を追加するイメージでしょうか? どこに追加するなどの具体的なイメージはありますか? (自分としてはメディアプレビューの下にチェックボックスを追加するのが一番普通かなと思いました)
はい、そんな感じで問題ないです~ デフォルトを有効にするか無効にするかはちょっと悩む。
- 一部ユーザーのアイコンが表示されない
これはバグだと思うので調べたいのですが再現するユーザがこちらで見つけられなかったので、発生しているユーザのnpubなどわかりましたら共有いただきたいです。
@ocknamo
- 一部ユーザーのアイコンが表示されない
これはバグだと思うので調べたいのですが再現するユーザがこちらで見つけられなかったので、発生しているユーザのnpubなどわかりましたら共有いただきたいです。
よくよく確認するとオリジナルの画像が取得できないようでした。(nostter.app で表示されてたのはブラウザキャッシュだったみたいです)
また、アイコン周りをリファクタリングして全てのアイコンの表示を ProfileIcon.svelte
に統一し、画像が取得できないときは猫アイコンが表示されるようになりました。 (#1300)
その兼ね合いでコンフリクトが発生しています。
お手数ですが都合のいいタイミングでマージしていただくか、必要あればこちらでマージしますのでメンション飛ばしていただければと思います。
また、アイコン周りをリファクタリングして全てのアイコンの表示を
ProfileIcon.svelte
に統一し、画像が取得できないときは猫アイコンが表示されるようになりました。 (#1300) その兼ね合いでコンフリクトが発生しています。 お手数ですが都合のいいタイミングでマージしていただくか、必要あればこちらでマージしますのでメンション飛ばしていただければと思います。
@SnowCait 連絡遅くなってすみません。 PRのコンフリクトはこちらで解消させたのですがオリジナル画像が取得できない場合の猫アイコンの表示機能が、プロキシ失敗時のオリジナル画像の表示と機能的にコンフリクトしており、svelte-remote-imageの方に機能を機能を追加する必要がありそうかなと考えています。
おそらくfailbackに指定したオリジナル画像もエラーになった場合にonError
のようなイベントを発火させる機能の追加が必要そうです。
ちょっとすぐ手を動かせる時間が取れるかわからないところがあり、これについてはしばらくお待ちいただくかもです。
@SnowCait 途中経過です。コミットが増えすぎたので一旦整理して、以下の課題に対応しました。
svelte-remote-image側でfallbackを配列で受け取れるようにしたり画像ロードの失敗判定を改善することで対応しています。
(ただしNostrの特性上どんな画像が与えられるかわからないため、すべてをカバーできていない可能性は否定できません^^;)
またこれは機能追加になりますがProfileIcon.svelte
に適応させる際に呼び出し元によってサイズが大きく変わってしまうのでs, m, lの引数を追加して画像サイズをコントロールできるように修正しました。
以下についてはWIPです。
NOTE: https://github.com/SnowCait/nostter/pull/1287/commits/5a2819f5f4032cfac6d8767809423102663c1d1b で画像プロキシの有効/無効を選べるオプションの設定を設定画面に追加しました
NOTE: https://github.com/SnowCait/nostter/pull/1287/commits/91c30f18047069077145787e3a83c30368e3bc91 svelte-remote-image v0.4.0 においてsafariで画像が一部表示されない問題をv0.4.1で修正
@SnowCait safariでプロフィールアイコンにレイアウトシフトが発生する問題を修正しました。 https://github.com/SnowCait/nostter/pull/1287/commits/0f2dc0ea52dd526206c68ddd05b97e1d272d73d1
@SnowCait ちなみにですが、現在svelte-remote-imageというライブラリで開発しているコンポーネントはnostterに取り込むことも可能です。 自分は現在の形でもいいと思うのですが、今後もし頻繁に不具合が発生する場合などには取り込んだほうがnostterのメンテナンスが楽になる可能性もあるため選択肢としては存在するということをお知らせしておきたかったです。
取り込んだ例: https://github.com/ocknamo/nostter/commit/43e77817e1cd6de8592033d336cac4565374ddeb
@SnowCait 213a0b42e746109f61602435771bcb46de9d7fda でアイコン画像の最適化処理を削除し、TLにはられる画像のみ対象となるように変更しました。
2bc309c6cd1150628270379ead1da1d8e7b082c1 srcsetの設定を削除しました。
@SnowCait is attempting to deploy a commit to the snowcait's projects Team on Vercel.
A member of the Team first needs to authorize it.
@SnowCait 遅くなりました。コードの指摘箇所修正しましたので時間あるときにご確認ください!
~Work in progress.~
画像最適化のためのproxyを設定しTLの画像とTLのユーザアイコンの画像のサイズを縮小して表示できるようにしました。
これにより、ユーザに以下の利益があります。
また以下が考慮されているためこの実装を取り込むことによるリスクは比較的少ないと考えています。
1. 画像の最適化の実装は標準的なsrcsetの実装です。
FYI. https://developer.mozilla.org/en-US/docs/Web/HTML/Element/picture#the_srcset_attribute
画像の読み込み中にぼかし効果(blur)のアニメーションを入れる以外は特殊な実装は行っていません(またblurアニメーションはオプションでいつでもオフにできます)
2. プロキシが何らかの理由で(WAFによるアクセス制限など)エラーを返す場合でもフロント側でfailbackにオリジナルのURLを設定できるためユーザの表示を保持できます。
failbackの実装はsvelte-remote-image内で実装しておりオプションを設定するだけで使用できます。
プロキシについて
cloudfleare Workersの無料枠を使ってプロキシ公開しており今回のPRにURLをハードコードしています。
Cloudflare Image OptimizationのTransform via URLとインターフェースを似せて作っているため(試したわけではありませんが)いつでも乗り換えが可能と考えられます。
実装はこちらにあります。 https://github.com/ocknamo/nostr-image-optimizer
対応画像タイプなどはREADMEを確認してください。
Workersの無料枠を使用していることから公開しているAPI自体には以下の制限があります。
有料枠に独自にデプロイする場合はWAFの設定を緩和可能になり、またcpuの実行時間も緩和されるためこれらの制限を取り除くことが可能になります。
影響範囲が大きいのでながながと書きましたが説明が足りない部分がありましたらなんでも聞いてください。よろしくおねがいします。