fjordllc / bootcamp

プログラマー向けEラーニングシステム
https://bootcamp.fjord.jp
MIT License
286 stars 71 forks source link

bookmark-button.vueをreact対応にする #5114

Closed komagata closed 7 months ago

komagata commented 2 years ago

下記を参考にしてreactに変える。

github-actions[bot] commented 2 years ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

github-actions[bot] commented 1 year ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

github-actions[bot] commented 1 year ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

github-actions[bot] commented 1 year ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

github-actions[bot] commented 1 year ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

github-actions[bot] commented 1 year ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

github-actions[bot] commented 1 year ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

github-actions[bot] commented 11 months ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

github-actions[bot] commented 9 months ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

kyokucho1989 commented 8 months ago

@komagata こんにちは。Issueの進め方について質問があります。

質問1 :book-select.vue のReact化について

book-select.vue ですが、すでにvueファイルはなく、以下のIssueでJSに置き換えられていました。 https://github.com/fjordllc/bootcamp/issues/4934

vueではなくなっているので、こちらはなにも変更を加えなくて構いませんか?

質問2: bookmark-button.vue の変更範囲について

bookmark-button.vue を参照しているファイルを確認すると、以下のファイルがありました。 app/javascript/components/question-edit.vue

ただ、これは以下のIssueで現在 @junohm410 さんが取り組んでいるものと変更範囲が被るかもしれません。 https://github.com/fjordllc/bootcamp/issues/7356

こちらはどこまで変更を行えば良いでしょうか。

junohm410 commented 8 months ago

@kyokucho1989 お疲れ様です。 ここ2日ほど用事や体調不良でFBCでの活動ができておらず、こちらにリアクションできておらず申し訳ないです🙇‍♂️

質問2: bookmark-button.vue の変更範囲について bookmark-button.vue を参照しているファイルを確認すると、以下のファイルがありました。 app/javascript/components/question-edit.vue ただ、これは以下のIssueで現在 junohm410 さんが取り組んでいるものと変更範囲が被るかもしれません。

はい、被りそうです。まずどのように被りそうかについて、今日中にここでシェアできるようにします。 (ちょっと一旦離席するので、取り急ぎご連絡というイメージです🙏)

すみませんが、よろしくお願いします。

junohm410 commented 8 months ago

@kyokucho1989

変更の概要

https://github.com/fjordllc/bootcamp/issues/7356PR(現時点ではドラフト)で、app/javascript/components/question-edit.vueは削除する予定です(非Vue化のIssueのため)。

同部分のブックマークボタンに関わる箇所は、下のRailsのviewファイルに移動させます。

https://github.com/fjordllc/bootcamp/blob/chore/convert-question-section-from-vue-to-slim/app/views/questions/_question_header.html.slim#L60

移動先では、現在では下のような形でbookmark-button.vueをマウントさせています。

https://github.com/fjordllc/bootcamp/blob/58b4a40d7d2a5c61c522a43e9bec28418505ca64/app/views/questions/_question_header.html.slim#L60

このマウント方法は、このQuestionのビューでの独自の手法ではなく、すでに他でbookmark-button.vueが使われている箇所と同じやり方です。 例: 現在のorigin/mainでのpagesでのマウント方法

https://github.com/fjordllc/bootcamp/blob/56bb9aeaff9577bfd1e64991b783343e1ffbf42e/app/views/pages/_doc_header.html.slim#L75

考えられる進め方について

React化のIssueなので、Questionに限らずですが、slimテンプレート上では下記のような実装に変わるのではないかと思っています。

# app/views/questions/_question_header.html.slim

= react_component('BookmarkButton', bookmarkable-id="#{question.id}", bookmarkable-type='Question')

私のPRは今日明日にはレビューに回せそうなので、kyokuchoさんがReact化を進めているうちにレビューが終わり、私のPRのマージ後に変更をrebaseで取り込んでもらう方法が、うまくいけば綺麗にすすめられる形かなと考えました。

のようにしてもらえると、不要なコンフリクトを減らせるのではないかなと考えました。

もちろん、React化にあたりpropsを変える必要が出てきそうとか、私のレビューが長くかかったとか、そのような際には都度相談させていただければと存じますので、やりとりを続けさせてもらえればというイメージです🙏

不明点あればDiscordで通話も大丈夫です。 取り急ぎご確認いただけると幸いです。よろしくお願いいたします🙏

kyokucho1989 commented 8 months ago

@junohm410 ありがとうございます! 一旦その方向で書いてみますー

junohm410 commented 8 months ago

@kyokucho1989 お疲れ様です。時間がかかってしまい申し訳なかったのですが、下のPRがマージされました。

https://github.com/fjordllc/bootcamp/blob/a917aa0c39957e5d1e3bd070efc2f307030bf31e/app/views/questions/_question_header.html.slim#L60

上のコメントでシェアした実装(既存のDocsなどと同じ形)でVueコンポーネントをマウントしています。 こちらを取り込んでいただき、ここをreact-railsのヘルパーに置き換えていただければいいのではと思います。 よろしくお願いいたします🙏

kyokucho1989 commented 7 months ago

本番環境で動作確認できましたのでクローズします。