-
Notifications
You must be signed in to change notification settings - Fork 71
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
選択必須の選択肢を選んだら「選択した理由」の入力欄を表示するようにした #5920
選択必須の選択肢を選んだら「選択した理由」の入力欄を表示するようにした #5920
Conversation
f1041e8
to
81a5b0a
Compare
50632ab
to
25cf769
Compare
@hikarook94 |
@AyakaTakashima |
@hikarook94 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AyakaTakashima
お疲れ様です!レビューしました!
動作確認はOKでした👍
コードについていくつかコメントさせていただいているのと、Bulletのエラーが画面に表示されておりN+1問題が発生しているかも?と思いました。
(アイコン?に隠れていてよく見えないですが...)
surveys_contorller.rb
でsurvey_questions
を取得するときに一度にまとめて取得する必要があるかなと思います!違っていたらすみません!
以上、ご確認よろしくお願いします🙏
app/javascript/survey.js
Outdated
answerRequiredChoice.name | ||
) | ||
const checkBoxStatuses = [] | ||
choicesInSameQuestion.forEach((ChoiceInSameQuestion) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ChoiceInSameQuestion
はキャメルケースのタイポ?だと思います!
app/javascript/survey.js
Outdated
if (answerRequiredChoice.checked) { | ||
additionalQuestionField[0].classList.remove('is-hidden') | ||
} else if ( | ||
!answerRequiredChoice.checked && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!answerRequiredChoice.checked
はあえて明示しなくても良いかも?と思いました!
@hikarook94 |
81a5b0a
to
786d6c9
Compare
ede4dfe
to
b9b4704
Compare
@hikarook94 左隅にある赤いWarningってクリックして中身開くことができるんですね...! 上記のエラーはshowページで発生しておりましたので、#5690 の方で修正させていただきました🙇♀️ 他のご指摘いただいた点も修正させていただきました! |
@AyakaTakashima
僕も「こんなのあったっけ?」って思いました😇 N+1問題は別PRで修正とのこと、了解しました! |
@hikarook94 |
@komagata |
786d6c9
to
7fd71bc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
conflictの解消をお願い致します〜。
b9b4704
to
644180b
Compare
7fd71bc
to
7f0eec5
Compare
644180b
to
44cefa9
Compare
@komagata |
app/javascript/survey.js
Outdated
@@ -0,0 +1,52 @@ | |||
document.addEventListener('DOMContentLoaded', () => { | |||
const Choices = Array.from(document.querySelectorAll('.a-toggle-checkbox')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
他のページでjsのエラーが出ていないかどうか調べてみていだだけるとありがたいです〜
(.a-toggle-checkboxやjs-additional_question_${choice.name}
存在しないページ)
c678478
to
ff85030
Compare
f0c6b5a
to
ee2aeae
Compare
@komagata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
確認させて頂きました。OKです〜🙆♂️
@komagata |
Issue
概要
アンケートのプレビュー画面(showページ)にて、「選択した理由」の回答が必須な選択肢を選んだ場合のみ、
追加の入力欄が現れるようにしました。
補足
アンケート機能自体が、完全な新機能なので補足させていただきます。
アンケート機能は、下記PRで実装しています。
関連PR: #5690
アンケート機能内で、質問を作成できる機能があります。
選択肢を選ぶ質問形式である時に、「選択した理由」の回答を必須にできるよう設定することができます。
質問を作成できる機能は、下記PRで実装されています。
関連PR: #5370
作成できる質問形式は全部で5つありますが、今回のPRで対象としている質問形式は下記の3つです。
変更確認方法
feature/display-input-fields-of-reason-for-choice-only-when-answer-required-choice-choosed
をローカルに取り込むbin/rails s
でローカルを立ち上げるhttp://localhost:3000/survey_questions
http://localhost:3000/surveys
変更前
ラジオボタン、チェックボックス、均等目盛、いずれも「選択した理由」の入力欄が常時表示されっぱなしでした。
変更後
ラジオボタン
チェックボックス
均等目盛