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

ページのページネーションをクリックしたら次のページの先頭に飛ぶ #4217

Merged
merged 1 commit into from
Mar 13, 2022

Conversation

chung3011
Copy link
Contributor

@chung3011 chung3011 commented Feb 13, 2022

#2677

要件

Vue.jsで表示されるページのページネーションをクリックしたら次のページの先頭に飛ぶようにする。

変更前

Screen Recording 2022-02-14 at 00 13 44 (1)

変更後

Screen Recording 2022-02-14 at 00 14 54

確認手順

  • feature/move-to-top-of-page-when-click-paginationブランチを手元に持ってくる。
  • ページネーションがあるページを確認する。
    http://localhost:3000/reports (日報リストページ)
    http://localhost:3000/products (提出リストページ)
    http://localhost:3000/users (ユーザーリストページ)
    http://localhost:3000/announcements (お知らせページ)
    http://localhost:3000/events (イベントリストページ)
    http://localhost:3000/searchables?document_type=all&page=3&word=a (検索の結果ページ)
    http://localhost:3000/talks (相談部屋ページ)
    http://localhost:3000/notifications (通知リストページ)
    http://localhost:3000/pages (Docsリストページ)
    http://localhost:3000/questions (Q&Aリストページ)
    http://localhost:3000/admin/companies (会社リストページ)
    http://localhost:3000/companies/company-id/users?target=all(会社の社員リストページ)
    http://localhost:3000/current_user/bookmarks(ユーザーのブックマークリストページ)
    http://localhost:3000/generations/:id(期生別によるユーザーのリストページ)
    http://localhost:3000/current_user/watches(Watch中ページ)

@chung3011 chung3011 requested a review from Saki-htr February 13, 2022 17:28
@chung3011
Copy link
Contributor Author

@Saki-htr さん、このPRをチェックしていただけませんか?

@Saki-htr
Copy link
Contributor

Saki-htr commented Feb 16, 2022

@chung3011 さん
レビュー依頼ありがとうございます🙆‍♀️

ページネーションがあるページを確認する。
http://localhost:3000/reportshttp://localhost:3000/productshttp://localhost:3000/users、等。

ページネーションがあるページがたくさん有り、チェックに漏れがあると怖いので、変更したページをすべて教えていただけると助かります🙏
お手数ですが、よろしくお願いいたします〜

@chung3011
Copy link
Contributor Author

chung3011 commented Feb 16, 2022

@Saki-htr さん

変更したページ

上記のページリストを更新しました。データが足りない場合、ページネーションは表示されません。
もう一度ご確認ください

Copy link
Contributor

@Saki-htr Saki-htr left a comment

Choose a reason for hiding this comment

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

@chung3011 さん
お待たせしてすみません🙇‍♀️
いくつか気になる点がありましたので、コメントさせていただきました〜。

また、descriptionに書いてくださった確認するページ一覧に漏れがあったり、書いていただいたURLと実際に変更したファイルが異なったりしていたのですが、レビュワーの方が、よりスムーズにレビューを行えるように、次回から気をつけていただけると良いかなと思いました〜😊

@@ -125,6 +125,7 @@ export default {
this.currentPage = pageNumber
this.getUsers()
history.pushState(null, null, this.newUrl(pageNumber))
window.scrollTo(0, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

こちら動作確認したところ、

  • ページネーションをクリックしたら次のページの先頭に飛ぶようにはなっていますが、
  • 全ページ同じユーザーが表示されました

以下に動作確認した際の動画をのせましたので、ご確認をお願いいたします〜

Image from Gyazo

Copy link
Contributor Author

@chung3011 chung3011 Feb 21, 2022

Choose a reason for hiding this comment

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

すみませんが、サーバーを再度実行していただけませんか?私はもう一度http://localhost:3000/usersをアクセスすると、その問題がありませんでした。
上の2GIFが実施結果です。

Copy link
Contributor

Choose a reason for hiding this comment

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

@chung3011 さん
調べたところ、@chung3011 さんの環境で成功し、私の環境で挙動がおかしくなる原因が分かりました。

原因

現在、最新のmainをgit pull --rebase origin mainで取り込んだ状態で、ユーザー一覧ページ( http://localhost:3000/users )を表示しようとすると、エラーが起こる状態になっています。(詳しい状況・解決方法はParuさんがこちらのチャットで説明してくださっています。)おそらく、chungさんのローカルのfeature/move-to-top-of-page-when-click-paginationブランチはこのエラーが起こるよりも前の状態なので、動作に問題が起こらなかったのだと思います。
komagataさんにレビュー依頼する際に、この事をお伝えしておくといいと思います〜☺️

レビューOKです🙆‍♀️

上記の問題を解消して動作確認を行ったところ、問題なく動きました!
私の勘違いで誤ったレビューをしてしまい、申し訳ありませんでした🙇‍♀️
私の方では、レビューOKとさせていただきます〜

@@ -159,6 +159,7 @@ export default {
url.searchParams.set('page', pageNumber)
}
history.pushState(history.state, '', url)
window.scrollTo(0, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

こちらのVueファイルですが、教えていただいた確認するページのうち

http://localhost:3000/companies

が該当のページかと思い動作確認しましたが、こちらのURLはVue.js化されていない&元々ページネーションが設定されていないようでした。bootcamp/app/views/index.html.slimを確認したところjsファイルは読み込まれていませんでした〜

調べたところ、app/javascript/admin_companies.vueの該当URLはhttp://localhost:3000/admin/companiesはでしたので、そちらを動作確認したところOKでした。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

descriptionを更新しました。

@@ -118,6 +118,7 @@ export default {
null,
location.pathname + (pageNumber === 1 ? '' : `?page=${pageNumber}`)
)
window.scrollTo(0, 0)
Copy link
Contributor

@Saki-htr Saki-htr Feb 20, 2022

Choose a reason for hiding this comment

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

こちらのファイルに該当するURLが、

ページネーションがあるページを確認する。

の一覧の中に入っていないようでした。
一覧に漏れがないと、レビューする時に動作確認すべきURLが分かるので助かります〜

http://localhost:3000/current_user/bookmarksを確認したところ、OKでした。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

descriptionを更新しました。

@@ -128,6 +128,7 @@ export default {
this.currentPage = pageNumber
this.getUsers()
history.pushState(null, null, this.newUrl(pageNumber))
window.scrollTo(0, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

こちらも

ページネーションがあるページを確認する。

の一覧から漏れているようです〜
http://localhost:3000/generations/:idを動作確認したところ、OKでした。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

descriptionを更新しました。

@@ -88,6 +88,7 @@ export default {
this.currentPage = pageNum
history.pushState(null, null, this.newURL)
this.getWatches()
window.scrollTo(0, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

こちらも

ページネーションがあるページを確認する。

の一覧から漏れているようです〜
該当するURLは http://localhost:3000/current_user/watches でしたので、そちらを動作確認したところ、挙動がおかしい所がいくつかありましたので、下記に詳細を書かせていただきました。
ご確認をお願いいたします〜

  • 1ページ目と最後のページ以外(下記動画でいうと2~4ページ目)のページをクリックすると、表示されません
    Image from Gyazo

  • 一番最後のページ(動画でいうと5ページ目)は、表示はされますが先頭に飛びません
    Image from Gyazo

  • 1ページ目以外のページ(動画でいうと2~5ページ)から1ページ目に飛ぶと、表示はされますが先頭ではなく真ん中に移動しました
    Image from Gyazo

Copy link
Contributor Author

@chung3011 chung3011 Feb 21, 2022

Choose a reason for hiding this comment

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

私はもう一度http://localhost:3000/current_user/watchesをアクセスすると、その問題がありませんでした。
sotugyouさんとしてログインします。
Screen Recording 2022-02-21 at 10 45 45
@Saki-htrさん、もう一度ご確認ください。

Copy link
Contributor

Choose a reason for hiding this comment

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

@chung3011 さん
再度確認させていただいたところ、問題なく動作しました!
私の環境でうまくいかなかった原因は、私がテストデータを増やすためにdb/fixtures/watches.yml の内容を変更していたことが原因でした。
お手数をおかけして、申し訳ありませんでした🙏

@chung3011
Copy link
Contributor Author

@komagataさん、このPRをチェックしていただけませんか?

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.

可能かわからないのですが、ページャーのコンポーネントで一括で対応できたりしないですかね?

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 ea1c945 into main Mar 13, 2022
@komagata komagata deleted the feature/move-to-top-of-page-when-click-pagination branch March 13, 2022 13:34
@github-actions github-actions bot mentioned this pull request Mar 13, 2022
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.

3 participants