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

Q&A Answer部分の非Vue化の不具合修正(追加) #8285

Merged
merged 3 commits into from
Jan 27, 2025

Conversation

reckyy
Copy link
Contributor

@reckyy reckyy commented Jan 21, 2025

Issue

概要

下記PRの動作確認を @komagata さんが行なっていたところ、エラーが出ているとお知らせいただいた。

answer.js:18 Uncaught TypeError: Cannot read properties of null (reading 'style')
    at HTMLDocument.<anonymous> (answer.js:18:1)

追記

同時に以下の不具合も見つけたので修正。

  • 回答がない場合、回答のローディングスケルトンがずっと表示される。
  • watchしていない質問に回答した後、watch状態にならない。

原因

一つ目(コンソールエラー)

下記で、回答要素(_answer.html.slimの要素群)を取得しています。
const answers = document.querySelectorAll('.answer')

回答がなければ、下記の処理は実行されないはずと考えていたが誤りでした。

  if (answers) {
    loadingContent.style.display = 'none'
    answerContent.style.display = 'block'

    answers.forEach((answer) => {
      initializeAnswer(answer)
    })
  }

document.querySelectorAllは該当する要素が存在しない場合でも、空のNodeListを返します。NodeListは常に真として評価されるため、if(answers)の条件は常に成立してしまい、意図した挙動になっていませんでした。
そのため、lengthを用いることで取得した回答数を確認するようにしました。

2つ目(回答がない場合、回答のローディングスケルトンがずっと表示される。)

一つ目の不具合を修正した後のコードが以下。

  if (answers.length > 0) {
    loadingContent.style.display = 'none'
    answerContent.style.display = 'block'

    answers.forEach((answer) => {
      initializeAnswer(answer)
    })
  }

この修正により、ローディングスケルトンと回答の切り替えが「回答があった場合」にのみ行われるようになったため、回答がない場合延々とスケルトンが表示されてしまいました。
そのため、この「ローディングスケルトンと回答の切り替え」を回答の有無に関わらず実行できるように、処理を分けました。

3つ目(watchしていない質問に回答した後、watch状態にならない。)

回答フォームの「コメントする」ボタンを押下すると、回答の作成や回答数の更新などが行われます。その処理の中には、質問のwatch状態を更新する処理も含まれています。
watch状態の更新は、以下の updateWatchable 関数で実現しています。

function updateWatchable(questionId) {
  store.dispatch('setWatchable', {
    watchableId: questionId,
    watchableType: 'Question'
  })
}

setWatchableは対象のwatch状態を取得して、更新してくれる処理です。
また、回答を作成すると、AnswerWatcher がその回答を自動的にwatch状態にします。

# app/controllers/api/answers_controller.rb
def create
  question = Question.find(params[:question_id])
  @answer = question.answers.new(answer_params)
  @answer.user = current_user
  if @answer.save
    Newspaper.publish(:answer_create, { answer: @answer }) # ここでAnswerWatcherの処理を発火してる

本来の想定フローは以下の通りです:

  1. 回答を作成する。この作成処理には、質問をwatch状態にする処理が含まれている。
  2. watch状態を取得して更新する。

しかし、回答の作成が非同期で行われていたため、作成処理が完了する前にwatch状態を取得していました。その結果、watch状態が正しく更新されない問題が発生していました。
そこで、回答の作成が完了したことを確認してから、watch状態の更新を行うように変更しました。

変更確認方法

  1. chore/handle-answerjs-warningをローカルに取り込む
  2. 質問詳細ページ以外のページを開く
  3. コンソールを確認し、エラーが出ていないことを確認

Screenshot

JSのコンソールエラー

変更前

スクリーンショット 2025-01-21 21 01 53

変更後

スクリーンショット 2025-01-21 21 02 23

回答がない場合、回答のローディングスケルトンがずっと表示される。

変更後

スクリーンショット 2025-01-27 10 46 21

watchしていない質問に回答した後、watch状態にならない。

変更前

2025-01-27.11.04.45.mov

変更後

2025-01-27.10.46.30.mov

@reckyy reckyy changed the title 条件の指定を正しくし、エラーを解消 JSのコンソールエラーを解消 Jan 21, 2025
@komagata
Copy link
Member

@reckyy Draftになっているようです。またテストが落ちているようです〜。

@reckyy
Copy link
Contributor Author

reckyy commented Jan 22, 2025

@komagata
ご確認ありがとうございます!

はい、Draftであることとテストが落ちていることは認識しております🙇‍♂️
CIが通り次第、正式にレビュー依頼させていただきますので、今しばらくお待ちいただけると幸いです!

@reckyy reckyy force-pushed the chore/handle-answerjs-warning branch from 87869fb to 919097d Compare January 23, 2025 02:51
@reckyy reckyy changed the title JSのコンソールエラーを解消 追加の不具合を解消 Jan 27, 2025
@reckyy reckyy changed the title 追加の不具合を解消 Q&A Answer部分の非Vue化の不具合修正(2つ目のPR) Jan 27, 2025
@reckyy reckyy changed the title Q&A Answer部分の非Vue化の不具合修正(2つ目のPR) Q&A Answer部分の非Vue化の不具合修正(追加) Jan 27, 2025
@reckyy reckyy marked this pull request as ready for review January 27, 2025 03:02
@reckyy reckyy self-assigned this Jan 27, 2025
Copy link
Member

@komagata komagata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確認させて頂きました。OKです〜🙆‍♂️

@komagata komagata merged commit 419ada3 into main Jan 27, 2025
8 checks passed
@komagata komagata deleted the chore/handle-answerjs-warning branch January 27, 2025 03:10
@github-actions github-actions bot mentioned this pull request Jan 27, 2025
32 tasks
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