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

[ グリッドカラム ] ツールバーからのリンク指定機能にnoreferrer、nofollow、link descriptionオプションを追加 #2367

Merged
merged 10 commits into from
Dec 28, 2024

Conversation

MasayaMORIMOTO
Copy link
Collaborator

@MasayaMORIMOTO MasayaMORIMOTO commented Dec 11, 2024

チケットへのリンク / 変更の理由(元のissueがあればリンクを貼り付ければOK)

#2199

どういう変更をしたか?

リンクツールバーに Edit link を追加し、a タグの rel やアクセシビリティ対応のテキストを設定できるようにしました。

・Add noreferrerと Add nofollow: rel に noreferrer と nofollow をそれぞれ設定できます。外すことも可能です。
・Accesibility link description: アクセシビリティ対応のテキスト設定にテキストを入れると、aria-label と visually-hidden の要素を持つspanにテキストが入ります。デフォルトでは 「ブロック名 + link」のテキストが設定されます。

スクリーンショットまたは動画

変更後 After

スクリーンショット 2024-12-11 15 50 56

実装者の確認事項

実装者はレビュワーに回す前に以下の事を確認してチェックをつけてください。

  • 複数の意図の変更 ( 機能の不具合修正 + 別の機能追加など ) を含んでいないか?
  • Files changed (変更ファイル)の内容は目視で確認したか?
  • readme.txt に変更内容は書いたか?
  • 本当にちゃんと確認をしたか?

プログラムの変更の場合

テストを書かないのは普通ではありません。書けるテストは極力書くようにしてください。

  • 書けそうなテストは書いたか?

変更内容について何を確認したか、どういう方法で確認をしたかなど

  • 編集画面でグリッドカラムアイテムのリンクツールバーの Edit link から Add noreferrer、Add nofollow、Accesibility link description が追加できることを確認しました。

  • 編集画面で設定した rel に noreferrer、nofollow、aria-label と visually-hidden の要素を持つ span に Accesibility link description に入力した内容がフロントエンドのソースで出力されていることを確認しました。

  • 編集画面で Edit link を設定後、設定を削除したときに、rel の noreferrer、nofollow が消えていることを確認しました。また、aria-label と visually-hidden の要素を持つ span に 「ブロック名 + link」のテキストが設定されます。

  • リンクだけ設定した場合は aria-label と visually-hidden の要素を持つ span に 「ブロック名 + link」のテキストが設定されることを確認しました。

  • aria-label属性が削除されていることを確認しました。

  • 「リンクを別ウィンドウで開く」のチェックがない場合、targetの出力がないことを確認しました。

  • 「リンクを別ウィンドウで開く」のチェックがある場合、targetが出力され、「_blank」が入ることを確認しました。

  • 「noreferrer を追加」「nofollow を追加」のチェックがない場合、relの出力がないことを確認しました。

  • 「noreferrer を追加」「nofollow を追加」のチェックがある場合、relが出力され、「noreferrer」「nofollow」がそれぞれ入ることを確認しました。

  • develop ブランチで作ったリンク設定付きのグリッドカラムアイテムブロックをこのの編集画面で開いた時「復旧を試みる」が出ないことを確認しました。

レビュワーに回す前の確認事項

  • 実装者はこのテンプレートのチェック項目をちゃんと確認してチェックしたか?

レビュワー確認方法・確認内容など

  • 実装者に同じ

レビュワー向け

レビュワーが確認して変更が反映されていない場合の確認事項

レビューしてみて意図した動作をしない場合は再度ビルドするなど以下の項目を確認してください。

  • プルしたか?
  • ビルドしたか?
  • ビルドしたディレクトリは正しいか(別の開発環境のディレクトリを見ていないか)?
  • npm install したか?
  • composer install したか?
  • キャッシュをクリアして確認したか?

@MasayaMORIMOTO MasayaMORIMOTO changed the title リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) 【確認待ち】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) Dec 11, 2024
@MasayaMORIMOTO MasayaMORIMOTO removed the request for review from ChiakiKouno December 11, 2024 08:45
@sysbird sysbird changed the title 【確認待ち】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) 【確認中】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) Dec 13, 2024
@sysbird
Copy link
Member

sysbird commented Dec 13, 2024

@MasayaMORIMOTO
対応ありがとうございます、
確認中です

  • dd noreferrer、Add nofollow、Accesibility link description が追加できることを確認しました

下記気になる点です

  • 過去のブロックを編集しようとしたときにリカバリーエラーが出ます
    (例) パターン サービスの流れ横並び_ビジネス全般 を develop ブランチ上でコピペしてグリッドカラムアイテムにリンクを設置していったん保存、このブランチに切り替えて編集(リロード)しようとしたときに発生します
  • src/blocks/_pro/grid-column-item/deprecated/index.js の blockAttributes4 のなかに今回追加した属性 (linkDescriptionなど) がありますが、これは次回の deprecated 時に反映されるのではないかと思います (私もこの部分いつも曖昧になってしまうのですが…) 前述のリカバリーエラーに関係あるかもしれません

@drill-lancer さんに確認してもらうとよいかと思います

@sysbird sysbird changed the title 【確認中】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) 【回答待ち】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) Dec 13, 2024
@sysbird
Copy link
Member

sysbird commented Dec 17, 2024

@MasayaMORIMOTO
リカバリーエラーが再現しないとのことでしたので、添付したグリッドカラムのコードを貼り付けてみていかがでしょうか?

  • develop ブランチで作成したブロックです
  • グリッドカラムの1番目のアイテムだけにリンクを設置してあります
  • 当ブランチに貼り付けて編集しようとするとエラーが発生します
  • 前回私が指摘したパターン サービスの流れ横並び_ビジネス全般 はグリックカラムカードでしたんで、私の間違えでした
  • アンカー属性のtargetrel が設定されてないときは 属性自体を出力しないようにできますでしょうか?
     → target=""rel=""のようになってるので不要かと〜
    grid-column-link.txt

@sysbird
Copy link
Member

sysbird commented Dec 20, 2024

@MasayaMORIMOTO
target=""rel="" について Outerブロックで対応していただいたので参考に〜
#2381

@MasayaMORIMOTO
Copy link
Collaborator Author

@sysbird
鳥さん、ご確認ありがとうございます。
ご指摘の件、対応できたと思いますので、お手数ですが再度ご確認をお願いします。
なお、「target=""、rel="" について」は、別プルリクで対応しようと思います。

@MasayaMORIMOTO MasayaMORIMOTO changed the title 【回答待ち】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) 【1人目鳥さん確認待ち】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) Dec 21, 2024
@sysbird
Copy link
Member

sysbird commented Dec 23, 2024

@MasayaMORIMOTO
リカバリーエラーが出なくなったのを確認しました
対応ありがとうございます!

細かいことで恐縮ですが、
Accesibility link description がある場合は、aria-label 属性はいらなくなるのでお願いできますでしょうか?

@sysbird sysbird changed the title 【1人目鳥さん確認待ち】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) 【回答待ち】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) Dec 23, 2024
@kurudrive kurudrive changed the title 【回答待ち】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) 【森本さん対応待ち】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) Dec 23, 2024
@MasayaMORIMOTO
Copy link
Collaborator Author

@sysbird
鳥さん、度々失礼します。
丸山さんに見ていただいて、「target=""、rel="" について」も
このプルリクで対応できることが分かりましたので、対応しました。
また、「Accesibility link description がある場合は、aria-label 属性はいらなくなる」件も
同時に対応しています。
何度もお手数ですが、ご確認をお願いいたします。

@MasayaMORIMOTO MasayaMORIMOTO changed the title 【森本さん対応待ち】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) 【1人目鳥さん確認待ち】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) &リンクの aria-label 属性削除 & targetとrel=が空の場合は出力しない方式に変更 Dec 24, 2024
@sysbird
Copy link
Member

sysbird commented Dec 25, 2024

@MasayaMORIMOTO

aria-label 属性削除 & targetとrel=が空の場合は出力しない方式に変更

について解決されていることを確認しました
対応ありがとうございます!

何度もすみませんが、2点気になるので確認お願いします
(1) developブランチで で作成したブロックで確認しています (前出のコメントで添付したコードです)

  • 左の子カラムは以前からリンクがあった箇所です → noreferrer と nofollow の項目がないです?
  • 中央の子カラムはこの ブランチでリンクを設定しました

もし、左は過去に設置したためリンクコンポーネントでこの仕様になってるようでしたら、教えてくださいね
grid-link

(2) discord で指摘ありましたように readme.txt を1項目にまとめますか?

@sysbird sysbird changed the title 【1人目鳥さん確認待ち】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) &リンクの aria-label 属性削除 & targetとrel=が空の場合は出力しない方式に変更 【1人目確認中→森本さん確認待ち】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) &リンクの aria-label 属性削除 & targetとrel=が空の場合は出力しない方式に変更 Dec 25, 2024
@kurudrive kurudrive changed the title 【1人目確認中→森本さん確認待ち】リンクツールバーに a タグのアクセシビリティや SEO に対応できるよう設定追加 (グリッドカラム) &リンクの aria-label 属性削除 & targetとrel=が空の場合は出力しない方式に変更 【1人目確認中→森本さん確認待ち】[ Grid Column (Pro) ] ツールバーからのリンク指定機能にnoreferrer、nofollow、link descriptionオプションを追加 Dec 26, 2024
@kurudrive kurudrive changed the title 【1人目確認中→森本さん確認待ち】[ Grid Column (Pro) ] ツールバーからのリンク指定機能にnoreferrer、nofollow、link descriptionオプションを追加 【1人目確認中→森本さん確認待ち】[ グリッドカラム (Pro) ] ツールバーからのリンク指定機能にnoreferrer、nofollow、link descriptionオプションを追加 Dec 26, 2024
@kurudrive kurudrive changed the title 【1人目確認中→森本さん確認待ち】[ グリッドカラム (Pro) ] ツールバーからのリンク指定機能にnoreferrer、nofollow、link descriptionオプションを追加 【1人目確認中→森本さん確認待ち】[ グリッドカラム ] ツールバーからのリンク指定機能にnoreferrer、nofollow、link descriptionオプションを追加 Dec 26, 2024
@MasayaMORIMOTO
Copy link
Collaborator Author

@sysbird
鳥さん、度々のご確認、ありがとうございます。
鳥さんの気になった点を対応しましたので、ご確認お願いします。

(1) 以前からある子カラムのリンク設定のポップアップを統一しました。
(2) readme.txt は石川さんのご指摘通り、1つにまとめました。

@sysbird
Copy link
Member

sysbird commented Dec 27, 2024

@MasayaMORIMOTO
動作問題ないことを確認しました
対応ありがとうございます!
たいへんお手数かけました

(2) readme.txt は石川さんのご指摘通り、1つにまとめました。

について2つめの
[ Specification change ][ Grid Column (Pro) ] Removed the aria-label...
が残ってる気がします
これも削除しちゃっていいんですよね?
本当にすみません

@sysbird sysbird changed the title 【1人目確認中→森本さん確認待ち】[ グリッドカラム ] ツールバーからのリンク指定機能にnoreferrer、nofollow、link descriptionオプションを追加 【1人目確認中→森本さん返信待ち】[ グリッドカラム ] ツールバーからのリンク指定機能にnoreferrer、nofollow、link descriptionオプションを追加 Dec 27, 2024
@MasayaMORIMOTO
Copy link
Collaborator Author

@sysbird
鳥さん、こちらこそ重ね重ね申し訳ありません。
readme.txt 修正しました。ご確認をお願いします。

@MasayaMORIMOTO MasayaMORIMOTO changed the title 【1人目確認中→森本さん返信待ち】[ グリッドカラム ] ツールバーからのリンク指定機能にnoreferrer、nofollow、link descriptionオプションを追加 【1人目確認中→鳥さん確認待ち】[ グリッドカラム ] ツールバーからのリンク指定機能にnoreferrer、nofollow、link descriptionオプションを追加 Dec 27, 2024
@sysbird
Copy link
Member

sysbird commented Dec 27, 2024

@MasayaMORIMOTO
修正ありがとうございます!
確認しました
お二人めに回しますね〜

@sysbird sysbird changed the title 【1人目確認中→鳥さん確認待ち】[ グリッドカラム ] ツールバーからのリンク指定機能にnoreferrer、nofollow、link descriptionオプションを追加 【2人目確認待ち】[ グリッドカラム ] ツールバーからのリンク指定機能にnoreferrer、nofollow、link descriptionオプションを追加 Dec 27, 2024
Copy link
Contributor

@akito-38 akito-38 left a comment

Choose a reason for hiding this comment

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

@MasayaMORIMOTO
こちら、指定いただいた手順で確認し、コードも見ました。
問題無いと思いますので、マージしておきます。

@akito-38 akito-38 changed the title 【2人目確認待ち】[ グリッドカラム ] ツールバーからのリンク指定機能にnoreferrer、nofollow、link descriptionオプションを追加 [ グリッドカラム ] ツールバーからのリンク指定機能にnoreferrer、nofollow、link descriptionオプションを追加 Dec 28, 2024
@akito-38 akito-38 merged commit 2eb6a62 into develop Dec 28, 2024
14 checks passed
@akito-38 akito-38 deleted the feature/grid-column/empty-atag-accessibility branch December 28, 2024 13:59
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