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

generation-users.vueと依存するVueファイルをReactコンポーネントに書き換え #7170

Merged
merged 27 commits into from
Apr 15, 2024

Conversation

Takuya-Sakai91
Copy link
Contributor

@Takuya-Sakai91 Takuya-Sakai91 commented Dec 29, 2023

Issue

概要

期生別のユーザー一覧ページと依存するVueファイルをReactに書き換え

変更確認方法

  1. chore/convert-generation-users-vue-component-to-reactをローカルに取り込む
  2. bin/setup を実行
  3. rails db:seedでページネーションの動作確認のため本PRで追加したユーザーデータをdevelopment環境に読み込む
  4. foreman start -f Procfile.devでサーバーを起動
  5. http://localhost:3000/にアクセスする
  6. 任意のユーザーでログインし、5期のユーザー一覧ページにアクセスする
  7. 以下、各リンクが正常に表示され、各遷移先画面が正常に表示されるかを確認する。
    • 各ユーザープロフィールページ
    • SNSリンクなどの表示
    • ユーザーに関連付けられたタグ
    • ユーザーのアクティビティ(日報、提出物、コメント、質問、回答)の数
    • プラクティスの進捗を表示するバー
    • フォロー機能
    • ページネーションが機能しているか(5期のユーザー一覧ページで)

Screenshot

  • 今回、見た目の変更としてはこちらのページネーションのアクティブ・非アクティブ化が未実装です。こちら別Issueとさせていただきます(参考
_development__5期のユーザー一覧___FBC

@Takuya-Sakai91 Takuya-Sakai91 self-assigned this Dec 29, 2023
@Takuya-Sakai91 Takuya-Sakai91 changed the title Chore/convert generation users vue component to react generation-users.vueと依存するVueファイルをReactコンポーネントに書き換え Dec 29, 2023
@Takuya-Sakai91 Takuya-Sakai91 force-pushed the chore/convert-generation-users-vue-component-to-react branch from 0e26169 to e648244 Compare January 9, 2024 15:00
@Takuya-Sakai91 Takuya-Sakai91 force-pushed the chore/convert-generation-users-vue-component-to-react branch 2 times, most recently from cbd7f30 to 27b1d67 Compare January 13, 2024 00:29
@Takuya-Sakai91 Takuya-Sakai91 force-pushed the chore/convert-generation-users-vue-component-to-react branch 2 times, most recently from 743880f to 46e9ff9 Compare January 26, 2024 14:11
@Takuya-Sakai91 Takuya-Sakai91 force-pushed the chore/convert-generation-users-vue-component-to-react branch 2 times, most recently from c0a8d7f to 5abca7c Compare February 18, 2024 15:23
@Takuya-Sakai91 Takuya-Sakai91 force-pushed the chore/convert-generation-users-vue-component-to-react branch from 5abca7c to 41b7bc2 Compare February 25, 2024 04:19
@Takuya-Sakai91
Copy link
Contributor Author

Takuya-Sakai91 commented Feb 25, 2024

@machida
generation-users.vueとそれに関連するファイルをReactに書き換えました。
以下の点について、本番環境と同様になるようスタイルの追加をお願いできますでしょうか。
もし自分が実装した箇所で不足しているところがあれば、教えていただきたいです。
お手数ですがご確認よろしくお願いいたします。

  • ローカル環境
  1. 最初と最後のページへのリンクを配置するため、ReactPaginateコンポーネントの前後にそれぞれのボタンを配置しています
_development__5期のユーザー一覧___FBC
  1. ユーザーごとのアイテム、両サイドの余白が追加されないのでご確認いただきたいです
_development__5期のユーザー一覧___FBC
  • 本番環境
30期のユーザー一覧___FBC 38期のユーザー一覧___FBC

@Takuya-Sakai91
Copy link
Contributor Author

@machida
上記いかがでしょうか。お手数ですがご確認よろしくお願いいたします。

@machida
Copy link
Member

machida commented Mar 5, 2024

@natsuto6 すいません、これから着手しますー

@machida
Copy link
Member

machida commented Mar 5, 2024

@natsuto6 デザイン崩れ修正しました。

このリンクはページャーの1ページ目だったら非アクティブに。
貼り付けた画像_2024_03_05_15_37

このリンクはページャーの最終ページだったら非アクティブに。
貼り付けた画像_2024_03_05_15_36

という動きも入れていただきたいですが、大変だったら別Issueでいいです。

@Takuya-Sakai91 Takuya-Sakai91 force-pushed the chore/convert-generation-users-vue-component-to-react branch from 716c41a to 0086807 Compare March 8, 2024 10:55
@Takuya-Sakai91
Copy link
Contributor Author

@machida
こちらご対応いただきありがとうございます。
実装が思いつかなかったので別Issueとさせていただければと思います。

@Takuya-Sakai91 Takuya-Sakai91 marked this pull request as ready for review March 9, 2024 00:18
@Takuya-Sakai91
Copy link
Contributor Author

@yocchan-git
お疲れ様です!こちらお手数ですがレビューをお願いできますでしょうか?
ご確認よろしくお願いします🙏

Copy link
Contributor

@yocchan-git yocchan-git left a comment

Choose a reason for hiding this comment

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

大枠はOKそうです!

ただ、フォローのリクエスト先が間違っていそうなので
そこの修正をお願いいたします🙏

Image from Gyazo

@yocchan-git
Copy link
Contributor

yocchan-git commented Mar 9, 2024

申し訳ないです。2点細かいですが修正お願いいたします

  1. Reactを読み込む際にbin/setupを変更確認方法の2あたりに追加していただけますと🙏
  2. ブランチ名(今回は修正の必要なしです)

ブランチ名はなんでもいいのですが、フィヨルドのルールに則った方がいいかなと🤔
こちらの記事を参考になさってください!

@Takuya-Sakai91
Copy link
Contributor Author

@yocchan-git
早々にご確認いただきありがとうございます!
フォロー機能修正しましたので再度ご確認お願いします🙏

指摘いただいた1点目について追加しました。

ブランチ名はなんでもいいのですが、フィヨルドのルールに則った方がいいかなと

こちら確かにそうかもしれないので以後気をつけます🙇‍♂️
ただ、Issueの参考ページだったり過去のプルリクエストとかを参考に作成しました👀

Copy link
Contributor

@yocchan-git yocchan-git left a comment

Choose a reason for hiding this comment

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

基本的な動作はバッチリです!

ただ、2点気になったので...🙇‍♂️

現状、下記のようにページネーションで遷移するとリンクの後ろに&page=2などが3つ,4つぐらいついてきます。
Image from Gyazo

これは意図する動作なのでしょうか?
(最後のpageだけを参照するとか、ただ、余計なparamsを増やすのはあまり良くない気がしますが...)

また、上記の例はたまたまpage=1が表示されないのでいいのですが、page=2&page=3などになるとバグになりそうなので、修正していただきたいです!🙏


また、page=2にいてもリロードしたら1に戻ってくるのですが、
こちらも意図する動作でしょうか?🤔

個人的には、リロードしても2にいて欲しいです笑
(というか、今もそのはずです!)

Image from Gyazo

ただ、このissue結構重く大変だったかなと思います!
修正になるとよりテンション下がるかと思いますが、よろしくお願いいたします🙇

@Takuya-Sakai91 Takuya-Sakai91 force-pushed the chore/convert-generation-users-vue-component-to-react branch from 20501b5 to 1fc6fd5 Compare March 10, 2024 11:03
@Takuya-Sakai91
Copy link
Contributor Author

@yocchan-git
ご指摘いただきありがとうございます!🙇‍♂️
ご指摘の2箇所の挙動を修正しました。お手数ですが再度ご確認をお願いいたします🙏

Copy link
Contributor

@yocchan-git yocchan-git left a comment

Choose a reason for hiding this comment

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

LGTM🎉

結構難しそうでしたが、すごいですね!
対応お疲れ様でした〜〜

@Takuya-Sakai91 Takuya-Sakai91 requested a review from komagata March 11, 2024 10:07
@Takuya-Sakai91 Takuya-Sakai91 force-pushed the chore/convert-generation-users-vue-component-to-react branch from 0fca47c to 6edb9c8 Compare April 8, 2024 01:35
@Takuya-Sakai91
Copy link
Contributor Author

@komagata
コンフリクトを解消しました。ご確認をお願い致します。

@komagata
Copy link
Member

komagata commented Apr 9, 2024

@natsuto6

こちらご対応いただきありがとうございます。 > 実装が思いつかなかったので別Issueとさせていただければと思います。

こちら、この別IssueはどのIssueになりますでしょうか?

@komagata
Copy link
Member

komagata commented Apr 9, 2024

@natsuto6 フォローするボタンを押すと下記のようになってしまうようです。

image

@Takuya-Sakai91
Copy link
Contributor Author

@komagata

こちら、この別IssueはどのIssueになりますでしょうか?

pagerのcomponentに変更したことにより、事象として発生しなくなっております。目的の実装になっているのでIssueとして立てておりませんでした。

@machida
komagataさんからの指摘内容について、こちらと同様の事象が発生しております。
本番環境においても同様の事象が発生しておりますが、mainブランチでは正常に動作しております。問題の修正は既に完了しており、machidaさんに対応いただくことはない認識でよろしかったでしょうか?必要であればご対応をお願いできればと思います。お手数ですがご確認をお願い致します🙏

@machida
Copy link
Member

machida commented Apr 10, 2024

@natsuto6 直しますー💪

@komagata
Copy link
Member

@natsuto6

pagerのcomponentに変更したことにより、事象として発生しなくなっております。目的の実装になっているのでIssueとして立てておりませんでした。

承知しました~。

@Takuya-Sakai91
Copy link
Contributor Author

@machida
ご対応いただきありがとうございます!フォローボタン押下時の事象が解消されていることを確認できました🙇‍♂️
@komagata
お手数ですが再レビューをお願い致します🙇‍♂️

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 e6f3de4 into main Apr 15, 2024
2 checks passed
@komagata komagata deleted the chore/convert-generation-users-vue-component-to-react branch April 15, 2024 22:58
@github-actions github-actions bot mentioned this pull request Apr 15, 2024
14 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.

5 participants