search

vektor-inc / vk-all-in-one-expansion-unit

wordpress plugin of powerful support
https://ja.wordpress.org/plugins/vk-all-in-one-expansion-unit/
GNU General Public License v2.0
7 stars 1 forks source link

[ カスタム投稿タイプ設定 ] Menu Icon設定を追加 #1086

Closed mtdkei closed 4 months ago

mtdkei commented 4 months ago

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

1085

どういう変更をしたか？

カスタム投稿タイプ設定のMenu Icon項目で、Dashiconsのiconから左メニューのアイコンを設定できるようにしました。 image

ソースコードについて

デザイン・UI

プログラムの変更の場合

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

その他

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

  1. ダッシュボードからカスタム投稿タイプ設定 > 新規投稿を追加 で新規追加
  2. 各種設定を行い、Menu Iconにあるアイコンを選択し保存
  3. 追加したカスタム投稿タイプの左メニューのアイコンが設定したアイコンになっていることを確認

また、以下も確認済みです。

確認URL

（　どこかのデモサイトかテストサーバーにデプロイ済みなどで確認できる場合はそのURL　）

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

  1. ダッシュボードからカスタム投稿タイプ設定 > 新規投稿を追加 で新規追加してください。
  2. 各種設定を行い、Menu Iconにあるアイコンを選択し保存してください。
  3. 追加したカスタム投稿タイプの左メニューのアイコンが設定したアイコンになっていることを確認してください。

また、以下の設定でもアイコンが変わっているか確認してみてください。

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

レビュワー向け

確認して変更が反映されていない場合の確認事項

mtdkei commented 4 months ago

~~あとはテストができれば確認いただけるのですが、PHPUnitでエラーになるのでDraftにしてます。 エラー内容はDiscodeに書いています~~ 解決したのでOpenにします。

drill-lancer commented 4 months ago

@mtdkei @kurudrive

以下 register_post_type の原文より

menu_icon string

The URL to the icon to be used for this menu. Pass a base64-encoded SVG using a data URI, which will be colored to match the color scheme — this should begin with 'data:image/svg+xml;base64,'.

Pass the name of a Dashicons helper class to use a font icon, e.g.'dashicons-chart-pie'.

Pass 'none' to leave div.wp-menu-image empty so an icon can be added via CSS.

Defaults to use the posts icon.

以下上記の Google 翻訳

メニューアイコン文字列

このメニューに使用されるアイコンの URL。データ URI を使用して、base64 でエンコードされた SVG を渡します。データ URI は、配色に合わせて色付けされます。これは、「data:image/svg+xml;base64,」で始まる必要があります。

フォント アイコンを使用するには、Dashcons ヘルパー クラスの名前を渡します。「dashicons-chart-pie」。

'none' を渡すと div.wp-menu-image が空のままになり、CSS 経由でアイコンを追加できるようになります。

デフォルトでは投稿アイコンが使用されます。

・・・とあるので最低限 'none' は許可したほうが良いような気がしますがいかがでしょうか？

mtdkei commented 4 months ago

@drill-lancer ご確認ありがとうございます。 noneが入るようにしました。

drill-lancer commented 4 months ago

@mtdkei 確認しました。特に問題は感じませんでした。

２人目確認お願いします。

kurudrive commented 4 months ago

確認しまっする

kurudrive commented 4 months ago

@mtdkei ありがとうございます。えーくせれんとー！

ですが...

これテストいらないかな（・ｗ・；

カスタムフィールドに保存した値をそのまま register_post_type() のパラメーターにわたすだけなので...。

で、ユニットテストは今回追加や変更したメソッドの返り値が想定した値を返すかどうかのテストなので、今回記載した内容だとテスト用につくったメソッドの返り値をテストしてるので残念ながら意味がないような空気を感じます（・ｗ・；

あえてやるなら、入力された値を保存する前の段階で有効な値以外無害化するメソッドを作って、 そのメソッドが正常に動作するかどうか...だったら最初からデフォルトのエスケープ関数使えばいいし、今回その手前の JS で弾いてる（ Good Job ）ので... テスト内で

  1. カスタム投稿タイプを登録する register_post_type() 走らせる
  2. 管理バーメニューの変数取得する
  3. その中身に今回追加したダッシュアイコンが含まれてるかどうか...

みたいな感じだけど...そもそもその前の投稿タイプが正常に登録されてるかどうかのテストから書かないといけなくなるし、今回の処理は冒頭述べた通り保存値をそのまま投げるだけなので、特にテスト書かなくても挙動に違いが出るわけではないので、テストはナシでOKデス。

でも実装内容＆テストを書こうとした心意気は非常に素晴らしいデス！

mtdkei commented 4 months ago

@kurudrive おっしゃる通り、保存値を渡すだけなので、そういう場合はテストが必要なわけではないのですね。 こちらのテストは削除しようと思いますがいかがでしょう？

kurudrive commented 4 months ago

@mtdkei はい、せっかく追加していただいてすみませんが今回のケースは削除でよろしくお願いいたします（汗

mtdkei commented 4 months ago

@kurudrive @drill-lancer ご確認ありがとうございました。 お二人に確認してもらったので私の方でマージしても大丈夫でしょうか？

kurudrive commented 4 months ago

@mtdkei はい！ありがとうございました！