fjordllc / bootcamp

プログラマー向けEラーニングシステム
https://bootcamp.fjord.jp
MIT License
286 stars 71 forks source link

Q&AのAnswer部分を非Vue化 #8033

Open reckyy opened 3 months ago

reckyy commented 3 months ago

Issue

概要

answer.vue, answer.vue、またこれらに関係があるファイルをまとめて非Vue化しました。

comment-placeholder.vuequestion-answer.vueの中で使用されていましたが、他の場所(comment.vue)で使用されていたので、削除はしていません。

スクリーンショット_2024-08-26_10_24_45

変更確認方法

  1. chore/answer-non-vue-conversionをローカルに取り込む
  2. komagataでログインする。
  3. 適当なページに飛び、以下の動作を確認。
    • 新規回答
    • [x] 回答数の更新
    • 既存の回答
    • [x] 内容の更新ができること
    • [x] ベストアンサーにする
    • [x] ベストアンサーを取り消す
    • [x] 削除する。その際、以下の事項も確認
    • それがベストアンサーなら、ベストアンサーを取り消す
    • 回答数の更新
    • 解決済みから、未解決に質問のステータスを変更
    • [x] 日付をクリックすると、AnswerのURLを取得

Screenshot

画面上の変更点はありません。

reckyy commented 3 months ago

変更点です。

reckyy commented 3 months ago

@motohiro-mm お疲れ様です! こちらのPRのレビューをお願いしたく、ご連絡いたしました。 お手隙の際にご対応いただけると嬉しいです。全く急ぎではないです。 もし、ご都合悪ければ仰ってください! よろしくお願いいたします。

motohiro-mm commented 3 months ago

@reckyy

承知しました! 1週間以内にレビューさせていただきます! よろしくお願いいたします!

reckyy commented 2 months ago

@machida (cc: @motohiro-mm ) お疲れ様です。 answerのpagerの実装についてご相談させていただきたいです。

相談したいこと

Pagerの追加を行わない方向で検討したいと考えています。

現在のPagerに関する課題

  1. 各answerのリンクの機能性

こちらは @motohiro-mm さんからのご指摘により、発覚しました。

2ページ目以降の回答の日付から取得できるURLに?page=2などページ数が含まれています。これは、後々回答数が増減して表示ページが変わった際にはリンクとして機能するのでしょうか?

例えば、回答が16件ある場合を想定します(1ページの上限は15件です)。

この問題に対して、現時点では適切な対応策が見つかっておりません。

  1. 回答が上限数ぴったりの時の場合の処理

こちらも同様に、 @motohiro-mm さんからのご指摘です。

途中のページで新規回答をすると、そのページの最下部(16個目)に表示されます。これは正しい挙動ですか? リロードするとそのページでは追加した回答は表示されなくなり、最終ページの一番最後に回答が追加されています。 途中ページで回答を追加する場合、「コメントする」をクリックしたあと最終ページに飛んで追加した回答が正しい位置に表示される、というのが正しい挙動なのかと思ったのですが、いかがでしょうか?

これを正しい仕様とする場合、次のような動作が期待されます。

前者のシチュエーションでは、回答数をカウントし、15件なら2ページ目に遷移させることで対応可能です。しかし、これでは操作の一貫性が失われる可能性があり、ユーザーにとって良い体験ではないと感じています。 また、現在の仕様(回答を非同期で編集できる)を変更することも、ユーザー目線で考えるとあまり良い選択ではないと考えています。

Pagerを追加しない理由

pagerの追加は、こちらのバグを防ぐためと認識しています。 しかし、開発および本番環境でスマホを使って回答数の多い質問を閲覧した際に、同様のエラーは再現されませんでした。 もし可能であれば、@machida さんから詳しいバグの再現手順を教えていただき、それを防ぐことができるか確認させていただければと思います。 それによって、Pagerの実装を取りやめることが可能かどうか、ご相談させていただきたく存じます。

お手隙の際に、ご確認いただけますと幸いです。 よろしくお願いいたします。

machida commented 2 months ago

@reckyy

なるほど!ありがとうございます。 コメントのUIにページャーをつけるとややこしくなりますね。 やはり、GitHubのように、数件ずつ読み込むのが良さそうです。

読み込みができなくなるエラーは、以前起こっていたのですが、 見返したところ、原因はJSで不要なリクエストが発生していたからっぽいです。なので、今回は相当な数がコメント(今回の場合はAnswer)がつかないとエラーは起きないと思うので、エラーが出るまでは対応はしない、という方針にしたいと思います。

なので、今回はページャーも数件ずつ読み込みをするのも無しでお願いします🙏 方針が変わってしまい申し訳ないです。

reckyy commented 2 months ago

@machida ご返信ありがとうございます! 委細承知しました。 とんでもないです!引き続きよろしくお願いいたします。

reckyy commented 2 months ago

@motohiro-mm レビューいただきありがとうございます! こちらのコメントですが、1と3はPagerの実装を取りやめたため問題が起こらなくなりました。 2に関しては、修正しました!→ https://github.com/fjordllc/bootcamp/pull/8033/commits/90751f6d6b0eb1dd2fef3bcfcdb1cb8c058b93d2 Toastの件はコメントしています。→ https://github.com/fjordllc/bootcamp/pull/8033#discussion_r1737647303

お手隙の際に、再度ご確認いただけますと幸いです。 よろしくお願いいたします。

reckyy commented 2 months ago

@motohiro-mm

ありがとうございます! ご指摘いただいた点を修正しましたので、お手隙の際にご確認いただけますと幸いです。

id属性をつけて特定の場所にジャンプできるURLを入力して一度そこにジャンプした後、同じページで移動し再度リロードしても元いた場所(移動した後の場所)に戻ってしまいURLで指定している場所にはジャンプされませんでした。

こちらですが、answer.vueの仕様をまたまた引き継げていませんでした。。 ブラウザはリロードした際スクロールしている位置を記憶し、その部分を表示します。 そのため、アンカーリンクにページ内ジャンプさせたい場合処理を書く必要がありました。 実際、answer.vueにも書いてました。

const answerAnchor = location.hash
if (answerAnchor) {
  this.$nextTick(() => {
    location.replace(location.href)
  })
}

なので、同様の処理を追加しました。 https://github.com/fjordllc/bootcamp/pull/8033/commits/8d9ab482c8adeb9e02be4acb06339db8d1fccb8a

reckyy commented 2 months ago

@motohiro-mm とても丁寧にレビューしていただき、助かりました😭 ありがとうございました!

@komagata お疲れ様です! チームメンバーレビューが終わりましたので、お手隙の際にレビューをよろしくお願いいたします。