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

SEO関連メタタグの設定 #4987

Closed
wants to merge 3 commits into from

Conversation

matsuoshi
Copy link
Contributor

@matsuoshi matsuoshi commented Mar 25, 2021

4.1 feature向けのPRです

概要(Overview・Refs Issue)

SEOやSNSシェア対応のため、metaタグ関連の以下を設定します

  • 全般
    • og:type
    • og:sitename
  • 商品詳細ページ (商品ごとに設定)
    • description および og:description
    • og:image
    • canonical および og:url

方針(Policy)

メタ関連の新規twigファイル meta.twig を作成しました。
4.0系では「ページ管理」のメタ設定 → metatag にてタグを設定していますが、タグのテンプレートとしては twig 側にあったほうがカスタマイズしやすいかと考えました。
(互換性のため、「ページ管理」の metatag に値が設定されている場合はそちらを優先し、空の場合のみ meta.twig ファイルを読み込むようにしています)

実装に関する補足(Appendix)

商品詳細の description と og:description は、以下から設定しています (上のものが優先される)

  • 商品説明 (一覧)
  • 商品説明
  • 「ページ管理」のメタ設定

テスト(Test)

Webテストを追加しています。

相談(Discussion)

デフォルトの og:image 画像を設定したいが、どのような実装方法がよいか

  • favicon のようにデフォルト画像をあらかじめ用意して metaタグに設定しておき、ファイル管理から差し替えられるようにする?
  • PDFロゴ画像のように、空白の画像をデフォルトとする? PDFのロゴ画像は空画像で良いのでは #4824
    その他よい方法がありましたら。

マイナーバージョン互換性保持のための制限事項チェックリスト

  • 既存機能の仕様変更
  • フックポイントの呼び出しタイミングの変更
  • フックポイントのパラメータの削除・データ型の変更
  • twigファイルに渡しているパラメータの削除・データ型の変更
  • Serviceクラスの公開関数の、引数の削除・データ型の変更
  • 入出力ファイル(CSVなど)のフォーマット変更

レビュワー確認項目

  • 動作確認
  • コードレビュー
  • E2E/Unit テスト確認(テストの追加・変更が必要かどうか)
  • 互換性が保持されているか
  • セキュリティ上の問題がないか

新機能追加に伴う競合確認

  • プラグインとの競合がないか
  • プラグインからのマイグレーションが必要か

@matsuoshi matsuoshi added enhancement 機能追加 affected:template フロントテンプレートの変更 labels Mar 25, 2021
@codecov-io
Copy link

Codecov Report

Merging #4987 (5b9fa24) into 4.1-feature (c46e77c) will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@                Coverage Diff                @@
##             4.1-feature    #4987      +/-   ##
=================================================
- Coverage          76.18%   76.14%   -0.05%     
  Complexity          6124     6124              
=================================================
  Files                437      437              
  Lines              20736    20736              
=================================================
- Hits               15798    15789       -9     
- Misses              4938     4947       +9     
Flag Coverage Δ Complexity Δ
tests 76.14% <ø> (-0.05%) 0.00 <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
src/Eccube/Form/Type/Admin/MemberType.php 75.00% <0.00%> (-20.84%) 11.00% <0.00%> (ø%)
.../Eccube/Doctrine/Common/CsvDataFixtures/Loader.php 79.48% <0.00%> (-2.57%) 16.00% <0.00%> (ø%)
src/Eccube/Service/OrderPdfService.php 95.03% <0.00%> (+0.35%) 51.00% <0.00%> (ø%)
src/Eccube/Entity/TaxRule.php 86.41% <0.00%> (+1.23%) 37.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c46e77c...5b9fa24. Read the comment docs.

@matsuoshi matsuoshi force-pushed the feature/seo-product-detail branch 2 times, most recently from 459c274 to b0f8d15 Compare March 29, 2021 07:52
@matsuoshi matsuoshi changed the title [WIP] SEO関連メタタグの設定 SEO関連メタタグの設定 Mar 30, 2021
@matsuoshi matsuoshi marked this pull request as ready for review March 30, 2021 02:33
@okazy
Copy link
Contributor

okazy commented Apr 6, 2021

@matsuoshi
Symfony 4.4 では $this->container->get() でリポジトリが取得できなくなるので先んじて対応しておきました。
ご確認とマージをお願いいたします。

matsuoshi#1

@okazy okazy mentioned this pull request Apr 6, 2021
5 tasks
metaタグのテストの Symfony4.4 対応
@@ -0,0 +1,28 @@
{% if Product is defined %}
Copy link
Contributor

Choose a reason for hiding this comment

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

twig変数の存在チェックだと、カスタマイズ時に誤動作しそうなので、商品一覧と詳細のルーティングで判定したほうがよいかもしれません。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ありがとうございます、 app.request.get('_route') で判定するように変更しました

{% elseif Category is defined %}
{% set meta_og_type = 'article' %}
{% set meta_description = Page.description %}
{% set meta_canonical = url('product_list', {'category_id': Category.id|default(null)}) %}
Copy link
Contributor

Choose a reason for hiding this comment

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

検索キーワードは含めないでよいですか?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

「検索キーワード」は管理画面で入力する meta keyword のことでしょうか、であれば従来どおり default_frame.twig で出力しており、今回は変更していません

Copy link
Contributor

Choose a reason for hiding this comment

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

@matsuoshi
フロントでのキーワード検索を行った際のcanonicalとog:urlですね。
同じcategory_idでもキーワード検索が入ると結果が変わってくるので
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chihiro-adachi はい、ここは意図したのもでして、一覧ページでは検索クエリ、ページ番号、orderby などは canonical に含めないようにしています
(カテゴリID以外のパラメタを canonical には指定せず、パラメタ違いで別ページと認識されるのを防ぐ)

Copy link
Contributor

Choose a reason for hiding this comment

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

ありがとうございます。了解しました。

@matsuoshi
Copy link
Contributor Author

@chihiro-adachi コメントありがとうございます、修正を入れています
統合しているほうのプルリクエスト #5019 にも反映しました

@matsuoshi matsuoshi changed the base branch from 4.1-feature to 4.1 July 6, 2021 07:09
@okazy okazy added this to the 4.1 milestone Aug 30, 2021
@okazy
Copy link
Contributor

okazy commented Aug 30, 2021

こちらの内容は #5019 にて取り込まれましたのでクローズします。

@okazy okazy closed this Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected:template フロントテンプレートの変更 enhancement 機能追加
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants