sadがついた日報の提出時にヘルプへ誘導するモーダルを表示 #4006

kaisumi · 2022-01-21T05:55:10Z

sadがついた日報を公開した後にヘルプへ誘導するポップアップを表示したい #3996

機能

sadがついた日報を提出する→日報の内容を表示する画面（show）にリダイレクト→モーダルが表示される。

_.FJORD.BOOT.CAMP.-.Google.Chrome.2022-02-16.08-52-42.mp4

kaisumi · 2022-01-21T07:29:53Z

@machida お疲れ様です。
先日ご相談していたsad日報でモーダル表示する件について、コードを書きました！
デザインをよろしくお願いいたします 😄

machida · 2022-01-24T08:12:47Z

了解ですー！！

sinsoku · 2022-01-30T08:32:45Z

app/controllers/reports_controller.rb

  end

  def notice_message(report)
    report.wip? ? '日報をWIPとして保存しました。' : '日報を保存しました。'
  end

+  def params_hash(report)
+    { notify_qa: report.wip? ? false : report.sad? }


三項演算子を使うより、 && で繋げた方が読みやすいかなと思いました。
（ただ、これは個人の好みかも）

Suggested change

{ notify_qa: report.wip? ? false : report.sad? }

{ notify_qa: !report.wip? && report.sad? }

@sinsoku コメントありがとうございます。おっしゃる通り条件式の方がスッキリしますね。変更しました 😄

machida · 2022-01-31T06:56:44Z

@kaisumi お待たせしました！！デザイン、そのままでほぼOKでしたー。少しだけ手を加えました。受講生にレビューお願いします🙏

kaisumi · 2022-01-31T07:04:50Z

@machida
デザインありがとうございました 😄

kaisumi · 2022-02-01T06:11:12Z

@Aseiide お疲れ様です！こちらお手隙の時にレビューお願いします 🙇

Aseiide · 2022-02-05T06:40:04Z

app/controllers/reports_controller.rb

  end

  def notice_message(report)
    report.wip? ? '日報をWIPとして保存しました。' : '日報を保存しました。'
  end

+  def params_hash(report)
+    { notify_qa: !report.wip? && report.sad? }


このnotify_qaというのはどういう意味でつけましたか?
このメソッドでtrueかfalseが返るのは理解できたのですが、単語の意味が分からなかったので

@Aseiide コメントありがとうございます 😄 notify_qaは「Q&Aをお知らせする」という意味でつけました。アドレスバーに表示され、ユーザーからも見える文字列なので、ユーザーに表示される内容とだいたい一致するようにしてあります。

@kaisumi
Q&Aへの誘導、というよりはヘルプページへの誘導なので、notify_helpとかindicate_helpとかのほうが則してるかなと思いました。
notify_qaだと/questionsに飛ぶのかなと勘違いしそうになりました。

@Aseiide コメントありがとうございます 😄 おっしゃる通りですね。誤認していました。修正しますー

Aseiide

レビュー遅くなりまして申し訳有りません。
1点だけコメントしました。
よろしくお願いします。
@kaisumi

Aseiide

@kaisumi
1件コメントしました。よろしくお願いいたします！

Aseiide · 2022-02-07T01:54:45Z

app/controllers/reports_controller.rb

  end

  def notice_message(report)
    report.wip? ? '日報をWIPとして保存しました。' : '日報を保存しました。'
  end

+  def params_hash(report)
+    { notify_qa: !report.wip? && report.sad? }


@kaisumi
Q&Aへの誘導、というよりはヘルプページへの誘導なので、notify_helpとかindicate_helpとかのほうが則してるかなと思いました。
notify_qaだと/questionsに飛ぶのかなと勘違いしそうになりました。

kaisumi

コメントありがとうございました！おっしゃる通りQ&Aとヘルプを混同していました。修正したので、確認をよろしくお願いします。

kaisumi · 2022-02-07T01:56:53Z

app/controllers/reports_controller.rb

  end

  def notice_message(report)
    report.wip? ? '日報をWIPとして保存しました。' : '日報を保存しました。'
  end

+  def params_hash(report)
+    { notify_qa: !report.wip? && report.sad? }


@Aseiide コメントありがとうございます 😄 おっしゃる通りですね。誤認していました。修正しますー

Aseiide

@kaisumi
1件コメントしました。よろしくお願いします。

Aseiide · 2022-02-07T05:36:23Z

app/views/reports/show.html.slim

@@ -170,3 +170,15 @@ header.page-header
                            | 提出物はまだありません。
        - else
          #js-reports(data-user-id="#{@report.user.id}" data-limit="10")
+
+- if params[:notify_help].eql?('true')
+    = render '/shared/modal', id: 'modal-notify-qa', title: '🍵 今日も学習お疲れ様でした！', auto_show: true


おそらく修正漏れだと思うのですが、id: 'modal-notify-help'に変更した方が良さそうです。

@Aseiide 修正漏れ失礼しました 🤕 修正したのでご確認をお願いしますー

Aseiide

LGTMです！

kaisumi · 2022-02-07T06:10:53Z

@Aseiide レビューありがとうございました！

@komagata お疲れ様です！お手すきの時にこちらレビューをお願いします 🙇

komagata · 2022-02-09T05:32:02Z

app/controllers/reports_controller.rb

@@ -63,7 +63,7 @@ def update
    @report.assign_attributes(report_params)
    canonicalize_learning_times(@report)
    if @report.save
-      redirect_to redirect_url(@report), notice: notice_message(@report)
+      redirect_to redirect_url(@report, params_hash(@report)), notice: notice_message(@report)


次のページでだけ有効な内容というのがflashを使って実装できるのでそちらを使う方がURLに影響しないので良いかもです〜

@komagata コメントありがとうございます！
flashで、ということは以下の緑のflashで表示したようなイメージですか？
#3996 (comment)

@kaisumi その見た目部品としてのflashではなく、railsのメソッドとしてのflashです〜。（次のページまでライフタイムのある変数）

@komagata railsメソッドですか！失礼しました、実装してみます 👍

kaisumi · 2022-02-10T13:05:42Z

以下の問題によりテストを通過できません：

日報のコメントの投稿日時をクリックするとコメントURLがコピーされる機能のテストが落ちる #4201

kaisumi · 2022-02-10T13:06:47Z

@komagata お疲れ様です。こちら修正してみました。レビューをよろしくお願いします 🙇

komagata · 2022-02-15T02:57:28Z

@kaisumi mainブランチや他のブランチはテスト通っているようです、そこの差分から調査してみてください〜

kaisumi · 2022-02-16T00:25:55Z

@komagata mainを取り込み、それに伴って生じた不具合箇所を修正しました。
ご確認をよろしくお願いします 🙏

komagata · 2022-02-16T03:57:10Z

test/system/emotions_test.rb

@@ -21,4 +21,23 @@ class EmotionsTest < ApplicationSystemTestCase
    assert_text '日報を保存しました。'
    assert_selector 'img#happy'
  end
+  test 'create a report with the sad emotion' do


テストとテストは1行開けた方がいいかもです〜

kaisumi · 2022-02-16T04:24:33Z

@komagata
コメントありがとうございます。改行追加しました。ご確認をお願いいたします 🙏

komagata

確認しました、OKですー🙆‍♂️

kaisumi self-assigned this Jan 21, 2022

kaisumi assigned machida Jan 21, 2022

sinsoku reviewed Jan 30, 2022

View reviewed changes

kaisumi requested a review from Aseiide February 1, 2022 06:11

kaisumi marked this pull request as ready for review February 1, 2022 06:11

machida removed their assignment Feb 1, 2022

Aseiide reviewed Feb 5, 2022

View reviewed changes

kaisumi requested a review from Aseiide February 6, 2022 23:25

kaisumi mentioned this pull request Feb 7, 2022

学習開始から100日毎に「おめでとう！えらい！」って通知が来ると嬉しい #3353

Closed

Aseiide reviewed Feb 7, 2022

View reviewed changes

kaisumi force-pushed the feature/show-popup-when-sad branch from bba7dce to 50ab34d Compare February 7, 2022 02:06

kaisumi commented Feb 7, 2022

View reviewed changes

kaisumi requested a review from Aseiide February 7, 2022 02:23

kaisumi mentioned this pull request Feb 7, 2022

100回目の日報を提出したタイミングで「おめでとう！」というメッセージをモーダルで表示 #4163

Merged

Aseiide reviewed Feb 7, 2022

View reviewed changes

kaisumi requested a review from Aseiide February 7, 2022 06:03

Aseiide approved these changes Feb 7, 2022

View reviewed changes

kaisumi requested a review from komagata February 7, 2022 06:11

komagata requested changes Feb 9, 2022

View reviewed changes

kaisumi force-pushed the feature/show-popup-when-sad branch from f5dc3fe to 995fe4d Compare February 10, 2022 12:46

kaisumi requested a review from komagata February 10, 2022 13:06

kaisumi and others added 9 commits February 15, 2022 12:43

sadで日報を保存した際にモーダルが表示される機能を追加

340b05e

メソッド名を変更、デフォルト値を追加

bce8f17

テストを作成（失敗するはずのテスト）

2de3b74

テストを修正（成功するはず）

effa2db

三項演算子を条件式に変更

b389c96

今日も学習お疲れ様でした！モーダルの文言を調整

f2f66f6

notify_qaをnotify_helpに修正

c906bd1

modal-notify-qaをmodal-notify-helpに修正

5a96a02

トリガーをparamsからflashに変更

9d1a6f3

kaisumi force-pushed the feature/show-popup-when-sad branch from 995fe4d to 9d1a6f3 Compare February 15, 2022 03:44

モーダルのタイトルが表示されない不具合を修正

c896625

komagata reviewed Feb 16, 2022

View reviewed changes

テスト間に改行を追加

8fb69c5

kaisumi requested a review from komagata February 16, 2022 04:25

komagata approved these changes Feb 16, 2022

View reviewed changes

komagata merged commit bfea773 into main Feb 16, 2022

komagata deleted the feature/show-popup-when-sad branch February 16, 2022 05:54

github-actions bot mentioned this pull request Feb 16, 2022

Release 2022-02-18 02:36:10 #4129

Merged

54 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sadがついた日報の提出時にヘルプへ誘導するモーダルを表示 #4006

sadがついた日報の提出時にヘルプへ誘導するモーダルを表示 #4006

kaisumi commented Jan 21, 2022 •

edited

Loading

kaisumi commented Jan 21, 2022

machida commented Jan 24, 2022

sinsoku Jan 30, 2022

kaisumi Jan 30, 2022

machida commented Jan 31, 2022

kaisumi commented Jan 31, 2022

kaisumi commented Feb 1, 2022

Aseiide Feb 5, 2022 •

edited

Loading

kaisumi Feb 6, 2022

Aseiide Feb 7, 2022

kaisumi Feb 7, 2022

Aseiide left a comment

Aseiide left a comment

Aseiide Feb 7, 2022

kaisumi left a comment

kaisumi Feb 7, 2022

Aseiide left a comment

Aseiide Feb 7, 2022

kaisumi Feb 7, 2022

Aseiide left a comment

kaisumi commented Feb 7, 2022

komagata Feb 9, 2022

kaisumi Feb 9, 2022

komagata Feb 9, 2022

kaisumi Feb 9, 2022

kaisumi commented Feb 10, 2022

kaisumi commented Feb 10, 2022

komagata commented Feb 15, 2022

kaisumi commented Feb 16, 2022

komagata Feb 16, 2022

kaisumi commented Feb 16, 2022

komagata left a comment

	{ notify_qa: report.wip? ? false : report.sad? }
	{ notify_qa: !report.wip? && report.sad? }

sadがついた日報の提出時にヘルプへ誘導するモーダルを表示 #4006

sadがついた日報の提出時にヘルプへ誘導するモーダルを表示 #4006

Conversation

kaisumi commented Jan 21, 2022 • edited Loading

機能

kaisumi commented Jan 21, 2022

machida commented Jan 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machida commented Jan 31, 2022

kaisumi commented Jan 31, 2022

kaisumi commented Feb 1, 2022

Aseiide Feb 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aseiide left a comment

Choose a reason for hiding this comment

Aseiide left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaisumi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aseiide left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aseiide left a comment

Choose a reason for hiding this comment

kaisumi commented Feb 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaisumi commented Feb 10, 2022

kaisumi commented Feb 10, 2022

komagata commented Feb 15, 2022

kaisumi commented Feb 16, 2022

Choose a reason for hiding this comment

kaisumi commented Feb 16, 2022

komagata left a comment

Choose a reason for hiding this comment

kaisumi commented Jan 21, 2022 •

edited

Loading

Aseiide Feb 5, 2022 •

edited

Loading