Skip to content

Conversation

akito-38
Copy link
Contributor

@akito-38 akito-38 commented Oct 22, 2024

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

#1115

どういう変更をしたか?

[ VK カテゴリー / カスタム分類リスト ウィジェット ] 昇順・降順を指定できるようにしました。

ソースコードについて

  • 複数の意図の変更 ( 機能の不具合修正 + 別の機能追加など ) を含んでいないか?
  • 関数名 / 変数名 / クラス名 / 保存値名 はそれだけで内容が想像できるものになっているか?紛らわしい命名になっていないか?
  • 関数名 / 変数名 / クラス名 / 保存値名 は既存のコードの命名規則に沿ったものになっているか?

デザイン・UI

  • 初見のユーザーが予備知識無しで使っても使いやすいようになっているか?
  • 情報意味を考慮した意味グルーピング・余白になっているか?
  • アラートの表示など追加した場合は他の同様の表示と同じデザインになっているか?

プログラムの変更の場合

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

  • 書けそうなテストは書いたか?
  • 表示要素が仕様通りに表示されない不具合の修正ではない or 表示要素に関する不具合修正の場合テストは書いたか?
    → スキップ。

その他

  • readme.txt に変更内容は書いたか?
  • Files changed (変更ファイル)の内容は目視でちゃんと確認したか?
  • このチェック項目を機械的にチェックするのではなく本当にちゃんと確認をしたか?
  • レビュワーが確認しないでリリースしてしまっても問題ないレベルまでちゃんと作りこみ・確認をしたか?

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

WP管理画面の外観→ウィジェットでVK カテゴリー/カスタム分類リストを設置する。
一番下の項目に「Display order」がある事を確認する。
ascending order(昇順)、descending order(降順)で正しく並び順が変わるか確認する。
フロント側でも同じように動作するか確認。

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

実装者と同じ確認をお願いいたします。

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

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

レビュワー向け

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

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

@MasayaMORIMOTO
Copy link
Contributor

MasayaMORIMOTO commented Oct 24, 2024

スクリーンショット 2024-10-24 10 20 28

昇順・降順の選択肢が英語のままとなってしまいました。
これはこちらの設定によるものでしょうか?確認をお願いします。
(そもそもこれは仕様で問題ない、でしょうか?)

@akito-38
Copy link
Contributor Author

@MasayaMORIMOTO @kurudrive @mthaichi
翻訳は別チケットと伺っているので、問題無い認識です。
いかがでしょうか?

@MasayaMORIMOTO
Copy link
Contributor

MasayaMORIMOTO commented Oct 24, 2024

@akito-38
返信ありがとうございました。
翻訳は別チケットということでしたら、実装・動作に問題ありません。
レビュー OK です。
2人目の方、よろしくお願いします!

@MasayaMORIMOTO MasayaMORIMOTO changed the title add:[カスタム分類リスト ウィジェット ] 昇順・降順を指定できるようにする 【2人目確認待ち】add:[カスタム分類リスト ウィジェット ] 昇順・降順を指定できるようにする Oct 24, 2024
@mthaichi
Copy link
Contributor

@MasayaMORIMOTO @kurudrive @mthaichi 翻訳は別チケットと伺っているので、問題無い認識です。 いかがでしょうか?

@akito-38 @MasayaMORIMOTO
はい、おっしゃるように翻訳はコンフリクトを起こしやすく対応が大変なので、別ブランチでお願いします。
というわけで、このブランチではラベルなどが英語になっていて、問題ありません。
ご対応ありがとうございます!

@kurudrive
Copy link
Member

@akito-38 @MasayaMORIMOTO .org に登録してる無料プラグインに関しては翻訳は .org 上で行うので、リポジトリでは翻訳しなくてOKですーん(・w・

@drill-lancer
Copy link
Member

drill-lancer commented Oct 25, 2024

@akito-38
実装ありがとうございます。

'orderby' => 'title', とありますが、title ではなく name のような気がします。
https://elearn.jp/wpman/function/wp_list_categories.html

@akito-38
Copy link
Contributor Author

@drill-lancer
ご指摘ありがとうございます。
nameとtitleの違いがよくわからなかったので調べてみました。

https://rishuntrading.co.jp/blog/php/sort_post_type_data/
上記サイトによると

'title' タイトルでソート
'name' Order by post name(post slug)

とあります。
nameはslug順ということなので、今回の場合はtitleが好ましいと思うのですがいかがでしょうか?

@drill-lancer
Copy link
Member

drill-lancer commented Oct 26, 2024

今回並び替えの対象となるのは投稿ではなくカテゴリ・タグ・カスタム分類(以下、タクソノミー)の中の分類項目(以下、ターム)です。
(どのような順番でタームを表示するかということです。)

ややこしいですが、投稿を並び替えるものとタクソノミーを並び替えるのは渡すものが違います。
今回はおそらく、タームを名前の昇順・降順で並び替えてようとしているので name が適切です。

P.S.
どうやらデフォルトでは name で並び替えるようなので、おそらく title での並び替えは無視されて name で並び替えられており、動作上は全く問題ない状態だと思われます。

@kurudrive kurudrive changed the title 【2人目確認待ち】add:[カスタム分類リスト ウィジェット ] 昇順・降順を指定できるようにする 【2人目確認中】add:[カスタム分類リスト ウィジェット ] 昇順・降順を指定できるようにする Oct 28, 2024
'hierarchical' => true,
'title_li' => '',
'taxonomy' => $instance['tax_name'],
'orderby' => 'title',
Copy link
Member

Choose a reason for hiding this comment

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

@MasayaMORIMOTO @drill-lancer

RICKさんご指摘の通り、
https://rishuntrading.co.jp/blog/php/sort_post_type_data/
の記事は投稿の並び替えの解説で、
今回はカテゴリーを取得する wp_list_categories() で、
wp_list_categories() の場合は タイトル順は name, slug 順は slug を使う感じなので、name の方が無難かなと。
https://developer.wordpress.org/reference/classes/wp_term_query/get_terms/

Suggested change
'orderby' => 'title',
'orderby' => 'name',

'taxonomy' => $instance['tax_name'],
'name' => $name,
'value_field' => 'slug',
'orderby' => 'title',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'orderby' => 'title',
'orderby' => 'name',

@akito-38
Copy link
Contributor Author

@drill-lancer @kurudrive
ご指摘ありがとうございます。理解しました。
修正しましたので、確認お願いいたします。

@kurudrive kurudrive changed the title 【2人目確認中】add:[カスタム分類リスト ウィジェット ] 昇順・降順を指定できるようにする [カスタム分類リスト ウィジェット ] 昇順・降順を指定できるようにする Oct 28, 2024
@kurudrive kurudrive merged commit 74672d3 into master Oct 28, 2024
3 checks passed
@kurudrive kurudrive deleted the feature/sort branch October 28, 2024 18:18
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.

5 participants