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

ブログのX共有時にタイトルが表示されるように修正 #7844

Merged
merged 2 commits into from
Jun 24, 2024

Conversation

Shrimprin
Copy link
Contributor

@Shrimprin Shrimprin commented Jun 10, 2024

Issue

概要

ブログをXで供した際に、Xの投稿にブログのタイトルが表示されるように修正しました。

変更確認方法

  1. fix/add-blog-title-on-x-sharingをローカルに取り込む
  2. bin/setupを実行
  3. foreman start -f Procfile.devでローカルサーバを立ち上げ
  4. 任意のアカウントでログインし、ブログ記事一覧にアクセス
  5. 任意のブログ記事をクリックし、詳細ページを開く
  6. Postするボタンをクリックし、下記を確認する
    • ブログ記事のタイトルがポストの最初に入力されている
    • ブログ記事のリンクがポストに表示されている
    • ハッシュタグ#fjordbootcampがポストに表示されている

Screenshot

変更前

  • タイトルが表示されていない
    image

変更後

  • タイトルが追加されている
    image

@Shrimprin Shrimprin self-assigned this Jun 10, 2024
@Shrimprin Shrimprin requested a review from motohiro-mm June 11, 2024 09:27
@Shrimprin Shrimprin marked this pull request as ready for review June 11, 2024 09:27
@Shrimprin
Copy link
Contributor Author

@motohiro-mm
お疲れ様です!本PRのレビューをお願いできますでしょうか 🙏
ご都合が悪いようでしたら遠慮なく仰ってください~
よろしくお願いいたします 🍵

@motohiro-mm
Copy link
Contributor

@Shrimprin
おつかれさまです!承知しました!💡

ただ初めてのレビューになるので、お時間いただきたいです🙏
来週水曜日までにはレビューさせていただきますが、よろしいでしょうか?💦
もし急ぎの場合には申し訳ありませんが、他の方にお願いいたします🙇‍♀️

大丈夫であればレビューさせていただきますので、よろしくお願いします!

@Shrimprin
Copy link
Contributor Author

@motohiro-mm
ありがとうございます 🙏
急ぎではまったくありませんのでゆっくりとご対応いただければかまいません 💡
もしも勝手にわからないところなどあればお気軽に聞いてください 😄

Copy link
Contributor

@motohiro-mm motohiro-mm left a comment

Choose a reason for hiding this comment

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

@Shrimprin
お疲れ様です!
以下の動作確認できました!✨
・ブログ記事のタイトルがポストの最初に入力されている
・ブログ記事のリンクがポストに表示されている
・ハッシュタグ#fjordbootcampがポストに表示されている

1点だけ気になったところをコメントしました!
ご確認をお願いします!🙏

@@ -3,6 +3,6 @@
li.share-buttons__item.is-x
= render 'share_button_x', article: article
li.share-buttons__item
= render 'share_button_facebook', article: article
= render 'share_button_facebook', article: article.id
Copy link
Contributor

Choose a reason for hiding this comment

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

Xのシェアボタン部分と同様に

article: article

articleをそのまま渡してerbファイル内でarticle.idとするか、
余計なデータを渡さずarticleidのみを渡したい場合は

article_id: article.id

としてerbファイルに渡すとわかりやすいかと思ったのですが、いかがでしょうか?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@motohiro-mm
レビューいただきありがとうございます!

余計なデータを渡さずarticleのidのみを渡したい場合は
article_id: article.id
としてerbファイルに渡すとわかりやすいかと思ったのですが、いかがでしょうか?

ご指摘いただいたようにした方が可読性が高まりそうなため修正いたしました。
お手数おかけしますが再度ご確認をお願いできますでしょうか 🙏

article.idのみをerbに渡しているため、erb側の変数をわかりやすくarticle_idに修正
@Shrimprin Shrimprin force-pushed the fix/add-blog-title-on-x-sharing branch from 5269d50 to 98b4819 Compare June 14, 2024 09:32
Copy link
Contributor

@motohiro-mm motohiro-mm left a comment

Choose a reason for hiding this comment

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

お疲れ様です!確認しました!
修正いただいた内容で問題ないと思いましたので、私からはApproveさせていただきます!👍

@Shrimprin
Copy link
Contributor Author

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

@komagata
メンバーからApproveいただきましたのでレビューをお願いできますでしょうか 🙏

@Shrimprin Shrimprin requested a review from komagata June 15, 2024 00:25
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 c296f83 into main Jun 24, 2024
5 checks passed
@komagata komagata deleted the fix/add-blog-title-on-x-sharing branch June 24, 2024 14:40
@github-actions github-actions bot mentioned this pull request Jun 24, 2024
13 tasks
@Shrimprin
Copy link
Contributor Author

@komagata
ありがとうございます!

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