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

「プログラミング経験」を複数選択可能なチェックボックス形式へ変更 #7970

Merged
merged 15 commits into from
Aug 21, 2024

Conversation

motohiro-mm
Copy link
Contributor

@motohiro-mm motohiro-mm commented Jul 22, 2024

Issue

概要

ユーザー情報の「プログラミング経験」を複数選択可能なチェックボックス形式へ変更しました。

  • 修正前
    セレクトボックス形式で1つのみ選択可能

  • 修正後
    チェックボックス形式で複数選択可能

変更確認方法

  1. feature/change_to_checkbox_and_add_experiencesをローカルに取り込む
  2. bin/rails db:resetを実行(migrateする&seedデータを入れるためです)
  3. foreman start -f Procfile.devでサーバーを立ち上げる
  4. hajime(現役生・研修生なら誰でもOK)でログイン
  5. 登録情報変更へ移動
  6. 「プログラミング経験」が複数選択可能なチェックボックス形式になっていることを確認
    (hajimeであれば「未経験」なのでチェックが入ってない状態で正しいです)
  7. 好きにチェックを変えて「更新する」をクリック
  8. komagata(メンターor管理者なら誰でもOK)でログイン
  9. さきほど登録情報を変更したユーザーのプロフィールページへ
  10. ユーザー非公開情報の「経験」の欄に先ほどチェックした選択肢が表示されていることを確認

※このブランチでbin/rails db:resetを実行した後、mainブランチへ戻り他の作業をする(元のデータベースに戻す)には以下を実行してください。
(migrateの履歴等を元に戻すためです)

  1. bin/rails db:rollbackを実行
  2. schema.rbが変更されてしまうので、それを戻す(git restore db/schema.rb)
  3. git checkout mainでmainブランチへ
  4. bin/rails db:resetを実行

お手数おかけしますが、よろしくお願いします。

Screenshot

変更前

image

変更後

スクリーンショット 2024-08-02 17 03 48

@motohiro-mm motohiro-mm self-assigned this Jul 22, 2024
@motohiro-mm
Copy link
Contributor Author

@machida

おつかれさまです!
2箇所デザインをお願いしたいところがあります。

1:チェックボックスのデザイン

issueにあるデザインは2列表示のチェックボックスだったので、デザインをお願いいたします。

該当ファイル↓
app/views/users/form/_experiences.html.slim

現状↓
スクリーンショット 2024-07-22 21 43 44

2:プロフィールページの「経験」部分のデザイン

現在プロフィールページの「経験」の項目には以下のように表示されるようにしています。
こちらも確認していただきデザインの追加・修正をお願いいたします。

該当ファイル↓
app/views/users/_user_secret_attributes.html.slim

現状:チェックなし(未経験)↓
スクリーンショット 2024-07-22 21 57 16

現状:チェックあり↓
スクリーンショット 2024-07-22 21 58 39

@machida
Copy link
Member

machida commented Jul 23, 2024

@motohiro-mm デザイン了解ですー。キャプチャを見た感じ良さそうですが、確認させていただきますー

@machida machida force-pushed the feature/change_to_checkbox_and_add_experiences branch from 125b559 to ebceab3 Compare July 31, 2024 08:41
@machida
Copy link
Member

machida commented Jul 31, 2024

@motohiro-mm おまたせしました:bowing_man:
デザインをちょこっといじり、フォーム内の別の場所の文章もちょこっとブラッシュアップしました。

最新の main を取り込んだので、

git pull --rebase origin feature/change_to_checkbox_and_add_experiences 

をお願いします。

@machida machida removed their assignment Jul 31, 2024
@motohiro-mm
Copy link
Contributor Author

@machida

お忙しい中、デザイン追加・修正ありがとうございました!✨
pull --rebase承知しました!

@motohiro-mm motohiro-mm marked this pull request as ready for review August 2, 2024 08:59
@@ -93,7 +94,7 @@ ja:
satisfaction: 満足度
opinion: ご意見
mail_notification: メール通知
job_seeker: 就職を希望する
job_seeker: 就職サポート
Copy link
Contributor Author

Choose a reason for hiding this comment

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

こちらの修正はデザイン修正時にmachidaさんに修正していただいた部分で、今回のissueには直接関係のない部分です。

@@ -18,7 +18,7 @@ komagata:
course: course1
job: office_worker
os: mac
experience: rails
experiences: 38
Copy link
Contributor Author

Choose a reason for hiding this comment

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

複数選択になった&選択肢が増えているので、既存ユーザーの経験の内容を少し変更し、複数選択しているユーザーがいる等のばらつきが出るようにしています。1人1人の変更に特に深い意味はありません。

@motohiro-mm
Copy link
Contributor Author

@nishitatsu-dev

おつかれさまです!
お忙しいところ恐縮ですが、こちらのPRのレビューをお願いできますでしょうか🙏
急ぎではありません!
ご都合が合わなければ、遠慮なくおっしゃってください🙇‍♀️
よろしくおねがいいたします!

@motohiro-mm
Copy link
Contributor Author

motohiro-mm commented Aug 2, 2024

レビューする上で留意していただきたい点を以下に記載しておきます!

以下のファイルの修正は、デザイン修正時にmachidaさんに修正していただいた部分で、今回のissueには直接関係のないファイルです。

  • app/views/users/form/_editor.html.slim
  • app/views/users/form/_job_seeker.html.slim
  • app/views/users/form/_os.html.slim
  • app/views/users/form/_sns.html.slim
  • config/locales/ja.ymlの97行目(コメントに書きました)

今回のissueでは、現行のUserモデルのexperienceカラムを新規作成したexperiencesカラムに移行しています。
データ移行のためのファイルはdb/data/20240718110343_copy_experience_to_experiences_for_user.rbです。
データ移行に関してはこちらのリンクを参考にしてください。
DBのデータをmigrateする方法 · fjordllc/bootcamp Wiki
bootcampでの過去のdata-migrationファイル
現行のexperienceカラムを削除するのは、こちらのPRマージ後、別issueで対応される予定です。
#7968

新規作成したexperiencesカラムは、active_flagというgemを使っています。
issueでの質問:実装方法について komagataさんの回答
kenn/active_flag: Bit array for ActiveRecord
Active Flagで効率的にフラグを実装する - アクトインディ開発者ブログ

現行のexperienceの選択肢がexperiencesのどの選択肢に移行しているのかはこちらをご確認いただければと思います。
issueでの質問:選択肢の対応について

他に気になる点・疑問等ありましたらご連絡ください!

@nishitatsu-dev
Copy link
Contributor

@motohiro-mm
レビュー承知しました〜
1週間以内に対応しますので、少々お待ちください🙏

@nishitatsu-dev
Copy link
Contributor

@motohiro-mm
まだ見始めた所なのですが、気になる部分がありました。ご確認お願いします🙏
(コードの確認は、このまま進めていきます)

  • チェックを入れる時、以下が周辺の選択部分と異なる
    • カーソルの形が、文字上で「矢印」になる
    • クリックに反応するのが、文字部だけ

@motohiro-mm
Copy link
Contributor Author

@nishitatsu-dev

早速確認してくださりありがとうございます!
その動きは私は把握できていませんでした!ご指摘ありがたいです🙏
私の方でも今から確認します🙇‍♀️

引き続きよろしくお願いいたします!

@motohiro-mm motohiro-mm force-pushed the feature/change_to_checkbox_and_add_experiences branch from ebceab3 to c22855c Compare August 4, 2024 15:52
@motohiro-mm
Copy link
Contributor Author

@machida

おつかれさまです!
上記コメントをいただいたので、私の方で少しデザインのクラス名の部分を少しさわりました!
該当コミット:c22855c
デザインに関する部分なのでご確認いただければと思います🙏
追加修正あればおっしゃってください🙇‍♀️
よろしくお願いします!

修正前↓

2024-08-05.0.48.32.mov

修正後↓

2024-08-05.0.47.42.mov

@machida
Copy link
Member

machida commented Aug 5, 2024

@motohiro-mm 確認しました。採用でお願いします!!対応ありがとうございます🙏

@motohiro-mm
Copy link
Contributor Author

@machida
早速ご確認いただきありがとうございました!
確認していただけて安心しました!このまま進めていきます!

Copy link
Contributor

@nishitatsu-dev nishitatsu-dev left a comment

Choose a reason for hiding this comment

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

@motohiro-mm
細かい話ですが、変数名other_ruby_and_javascriptを以下に変えた方が良さそうです〜
except_ruby_and_javascript

で、他は問題ないので、approveします!!
上記修正後、そのまま次のレビューに進んでいただいてOKです。

留意点と説明のリンクありがとうございました🙏
迷子にならずに済みました😅

@motohiro-mm
Copy link
Contributor Author

@nishitatsu-dev

ご確認いただきありがとうございました!

細かい話ですが、変数名other_ruby_and_javascriptを以下に変えた方が良さそうです〜
except_ruby_and_javascript

なるほど、私は英語が得意でないのでそのようなご意見助かります🙏

たしかに「except」を用いることで「RubyやJavaScriptを除いた他の言語」の意味がつよくなりますね!
ただ今回複数選択ができるので、「Ruby」を選択しつつ「Ruby、JS以外の経験あり」という選択もあり得ます。すると「except」がもっている除外のイメージがちょっと気になりました。
次回の開発MTGでチームメンバーの意見を伺ってみて、検討させていただこうと思います🙇‍♀️

自分の英語の語彙が少ないため、ご意見いただいて調べてみてとても勉強になりました!
ありがとうございます😄

@motohiro-mm motohiro-mm force-pushed the feature/change_to_checkbox_and_add_experiences branch from c22855c to 22b73d2 Compare August 7, 2024 09:08
@motohiro-mm
Copy link
Contributor Author

@nishitatsu-dev

開発MTGで相談したところ、複数選択でrubyなどを選択できることから、otherの方が良いのではないかという話になりました!
other_ruby_and_javascriptではなくother_languagesに修正しました!
レビューありがとうございました🙏

@motohiro-mm
Copy link
Contributor Author

motohiro-mm commented Aug 7, 2024

@komagata

おつかれさまです!
レビューをお願いいたします!

開発MTGでご相談させていただいた、other_ruby_and_javascriptは「他のRuby」となり第二のRubyを表すような言葉になると考え、other_languagesに変更いたしました。

よろしくお願いいたします!

@komagata
Copy link
Member

komagata commented Aug 8, 2024

@motohiro-mm

other_languagesに変更いたしました。

other_languagesだとコンテキストがないので以外なのかがわからないかな〜と思いました。

@motohiro-mm
Copy link
Contributor Author

motohiro-mm commented Aug 9, 2024

@komagata

other_languagesだとコンテキストがないので何以外なのかがわからないかな〜と思いました。

ではlanguages_other_than_ruby_and_javascriptはどうでしょうか?
決定し次第コードを修正いたします🙏

追記:
other_languageslanguages_other_than_ruby_and_javascriptに修正しました!

@thmz337
Copy link
Contributor

thmz337 commented Aug 14, 2024

@motohiro-mm

お疲れ様です。現在issue#7968の対応を行っています。
対応箇所の検索を行っていたのですが、以下画像赤枠の箇所を見つけました。

スクリーンショット 2024-08-14 23 27 45

これらはこちらのPRでの対応という認識で良いでしょうか。

ご確認よろしくお願いいたします。

@motohiro-mm
Copy link
Contributor Author

@thmz337

ご連絡ありがとうございます!
詳細確認はまだですが、私の方で対応します🙏

@motohiro-mm motohiro-mm force-pushed the feature/change_to_checkbox_and_add_experiences branch from 22b73d2 to e6603ab Compare August 15, 2024 06:23
@@ -1,4 +1,4 @@
json.(user, :id, :login_name, :name, :description, :github_account, :twitter_account, :facebook_url, :blog_url, :job_seeker, :free, :job, :os, :experience, :email, :roles, :primary_role, :icon_title, :cached_completed_percentage, :completed_fraction, :graduated_on)
json.(user, :id, :login_name, :name, :description, :github_account, :twitter_account, :facebook_url, :blog_url, :job_seeker, :free, :job, :os, :email, :roles, :primary_role, :icon_title, :cached_completed_percentage, :completed_fraction, :graduated_on)
Copy link
Contributor Author

@motohiro-mm motohiro-mm Aug 15, 2024

Choose a reason for hiding this comment

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

apiで既存のexperienceカラムは使用されていませんでしたので、experiencesカラムに修正せず削除するようにしました。
dfda897

Copy link
Contributor

Choose a reason for hiding this comment

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

@motohiro-mm
ありがとうございます!

@motohiro-mm
Copy link
Contributor Author

@komagata

追加修正しましたので、再度ご確認をお願いいたします!

other_languageslanguages_other_than_ruby_and_javascriptに修正しています。
e6603ab
そちらも確認をお願いいたします🙏

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です〜🙆‍♂️

@@ -0,0 +1,23 @@
# frozen_string_literal: true

class CopyExperienceToExperiencesForUser < ActiveRecord::Migration[6.1]
Copy link
Member

Choose a reason for hiding this comment

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

👍

@komagata komagata merged commit ceb3a2b into main Aug 21, 2024
4 checks passed
@komagata komagata deleted the feature/change_to_checkbox_and_add_experiences branch August 21, 2024 00:12
@github-actions github-actions bot mentioned this pull request Aug 21, 2024
17 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