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

相談部屋個別ページを開いたら、最新のコメントの位置までスクロールする #4592

Conversation

clio209
Copy link
Contributor

@clio209 clio209 commented Apr 8, 2022

issue #4437

概要

相談部屋個別ページを開いたら、最新のコメントの位置までスクロールするようにしました。

変更前

_development__muryouさんの相談部屋___FJORD_BOOT_CAMP(フィヨルドブートキャンプ)

ユーザーの個別相談ページに行くと、ページの一番上が表示されます。

変更後

_development__muryouさんの相談部屋___FJORD_BOOT_CAMP(フィヨルドブートキャンプ)_と_satoshisuto_—_-zsh_—_80×24

ユーザーの個別相談ページに行くと、ページの最新のコメントの位置にスクロールします。

ローカル環境での確認方法

  1. ブランチ feature/add_scroll_down_to_latest_comment_when_open_user_talks_page をローカルに取り込みます。
  2. admin権限を持つユーザーでログインし、コメントがあるユーザーの個別相談ページをクリックすると、最新のコメントの位置までスクロールすることを確認します。

@clio209 clio209 requested a review from NorifumiOgawa April 9, 2022 04:45
@clio209
Copy link
Contributor Author

clio209 commented Apr 9, 2022

@NorifumiOgawa さん
お忙しいところ恐縮ですが、お時間ある際にレビューいただけますと幸いです🙇‍♂️
なお、テストは、本内容がそもそもメソッド等を作成していない程度のものであること、また他のスクロール系のPRに、テストがないことから、一旦作成はしておりません。質問等、なんでも仰っていただければ幸いです。

@clio209 clio209 marked this pull request as ready for review April 9, 2022 04:50
@clio209 clio209 self-assigned this Apr 9, 2022
@NorifumiOgawa
Copy link
Contributor

@clio209 さん、レビュー遅くなってすみません!🙏

コードの修正内容を確認し、ブランチをローカルに落として動作確認してみました。
コードの修正も動作も特に違和感は感じませんでしたが、1点仕様の面で教えて下さい。

  • この仕様追加の対象となるのは管理者のみでしょうか?

このPRの元のIssueにも特に管理者のみとか記述されていないようでしたが、例えば「hatsuno」さんが左メニューの相談タブをクリックしても最新コメントまでスクロールしなかったので、その点だけあれ?と思ったので教えて下さい。
よろしくお願いします。

@clio209 clio209 force-pushed the feature/add_scroll_down_to_latest_comment_when_open_user_talks_page branch from 70e9025 to d1489d1 Compare April 11, 2022 23:43
@clio209
Copy link
Contributor Author

clio209 commented Apr 11, 2022

@NorifumiOgawa さん
お忙しい中、レビューm本当にありがとうございます🙌
仰る通り、ユーザーの方のナビからのリンクにはanchorが抜けておりました💦💦
anchorをつけて、最新のコメントに行くようにしましたので、お手数ですが再度ご確認いただけますと幸いです🙇‍♂️

Copy link
Contributor

@NorifumiOgawa NorifumiOgawa left a comment

Choose a reason for hiding this comment

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

修正内容を確認しました。LGTMです😊

@clio209
Copy link
Contributor Author

clio209 commented Apr 12, 2022

@NorifumiOgawa さん
レビュー、コメントいただき、本当に有難うございます!
@komagata さん
お忙しいところ恐縮ですが、お時間ある際にレビューいただけますと幸いです🙇‍♂️

@@ -2,6 +2,6 @@

module TalksHelper
def unreplied_index_path(talk)
admin_login? ? talks_unreplied_index_path : talk_path(talk)
admin_login? ? talks_unreplied_index_path : talk_path(talk, anchor: 'latest-comment')
Copy link
Member

Choose a reason for hiding this comment

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

これってコメントが一件も無い場合でもlatest-commentってanchorはありますか?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@komagata そのanchorはない状態になっています。。

Copy link
Member

Choose a reason for hiding this comment

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

@clio209 コメントが一件もない状態でも大丈夫な感じにしたいですね〜。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@komagata コメントが一件もない場合もanchorがあるようにしたのですが、意図とあってますでしょうか・・??

@clio209 clio209 force-pushed the feature/add_scroll_down_to_latest_comment_when_open_user_talks_page branch from 3e8af89 to 557d9ee Compare April 18, 2022 09:28
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 a5acc59 into main Apr 20, 2022
@komagata komagata deleted the feature/add_scroll_down_to_latest_comment_when_open_user_talks_page branch April 20, 2022 05:02
@github-actions github-actions bot mentioned this pull request Apr 20, 2022
20 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.

3 participants