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

検索結果一覧でWIPの日報やDocsを識別できるようにした #4501

Merged
merged 9 commits into from Apr 17, 2022

Conversation

eatplaynap
Copy link
Contributor

@eatplaynap eatplaynap commented Mar 25, 2022

Issue概要

検索結果一覧では、見た目上WIPとそうでないものとの区別がつかないため、WIPであることを見て分かるように修正しました。

変更前

image
WIPとWIPでないものが混在しているが、どれがWIPか分からない

変更後

image
WIPのものがひと目でわかるようになった

確認手順

  1. feature/identify-wip-from-search-resultsを手元に取り込む
  2. bin/rails setup実行
  3. bin/rails sでサーバーを立ち上げる
  4. 好きなユーザーでログインし、好きな文字列でWIPとWIPでない日報を作成する
  5. 検索フォームから先ほど作成した日報に記載の文字列で検索を行い、検索画面一覧からWIPとWIPでない日報の区別がつくことを確認する

@eatplaynap
Copy link
Contributor Author

@clio209
お疲れさまです!お手すきの際にこちらのコードのレビューをお願いいたします:pray:

@eatplaynap eatplaynap marked this pull request as ready for review March 28, 2022 10:29
@eatplaynap eatplaynap force-pushed the feature/identify-wip-from-search-results branch from a96529c to da3ad52 Compare March 31, 2022 06:27
@eatplaynap eatplaynap requested a review from clio209 March 31, 2022 08:32
Copy link
Contributor

@clio209 clio209 left a comment

Choose a reason for hiding this comment

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

@eatplaynap さん
issue取り組みお疲れ様です。再度取り込み、挙動を確認しました。
また、if文のリファクタリング、有難うございます。非常に読みやすくなりました!all等は深い意味はなく、何かそうしないといけない理由を考えたまでで、混乱させて申し訳ないです。。🙇‍♂️
自分の方はapproveいたします〜。

@eatplaynap
Copy link
Contributor Author

@clio209
レビューありがとうございました:smile:
本田さんが質問してくださったおかげでコード全体がめちゃくちゃスッキリしました〜!

@cafedomancer
お手すきの際にチェックお願いいたします:pray:

app/javascript/searchable.vue Show resolved Hide resolved
app/javascript/searchable.vue Show resolved Hide resolved
app/views/api/searchables/_searchable.json.jbuilder Outdated Show resolved Hide resolved
@eatplaynap eatplaynap force-pushed the feature/identify-wip-from-search-results branch from 60e9788 to db40874 Compare April 7, 2022 08:17
@eatplaynap
Copy link
Contributor Author

@cafedomancer
ご確認ありがとうございました!
すみません、Railsのすごく初歩的なことを理解できていない気がするのですが、wip?についてご教示いただけるとありがたいです:pray:

Copy link
Contributor

@cafedomancer cafedomancer left a comment

Choose a reason for hiding this comment

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

🌟

@eatplaynap
Copy link
Contributor Author

@cafedomancer 丁寧なレビューありがとうございました!勉強になりました〜!
@komagata チームメンバーの確認OKをいただいたので、お手すきの際にレビューをお願いいたします:pray:

@eatplaynap eatplaynap requested a review from komagata April 7, 2022 08:59
Comment on lines 8 to 10
if searchable.respond_to?(:wip?)
json.wip searchable.wip?
end
Copy link
Member

Choose a reason for hiding this comment

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

APIとしてはwipがfalseの場合も知りたいと思うので、項目自体を表示しないのではなく、falseを出すのがいいと思います〜

@eatplaynap
Copy link
Contributor Author

@komagata
ご確認ありがとうございます!修正したので再度チェックをお願いいたします〜!

@eatplaynap eatplaynap requested a review from komagata April 8, 2022 08:39
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 d7a43bc into main Apr 17, 2022
@komagata komagata deleted the feature/identify-wip-from-search-results branch April 17, 2022 01:53
@github-actions github-actions bot mentioned this pull request Apr 17, 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.

None yet

4 participants