Problem/Motivation

CategorizingPluginManagerInterface has changed in Drupal 12:
https://git.drupalcode.org/project/drupal/-/commit/1e719791c181ab2d50e6b...

-  public function getSortedDefinitions(?array $definitions = NULL): array {
+  public function getSortedDefinitions(?array $definitions = NULL, string $label_key = 'label'): array {
     $definitions = $definitions ?? $this->getDefinitions();

-  public function getGroupedDefinitions(?array $definitions = NULL): array {
+  public function getGroupedDefinitions(?array $definitions = NULL, string $label_key = 'label'): array {

Proposed resolution

Can we be compatible with Drupal 12 whil staying compatible with Drupal 11 ? And Drupal 10 ? There is still 2 Drupal 10 for every Drupal 11: https://new.drupal.org/project/usage/drupal

For information, Core is already implementing CategorizingPluginManagerInterface since Drupal 11.2 (#3474533: ComponentPluginManager must implement CategorizingPluginManagerInterface) so our implementation is needed only for Drupal 10.

Issue fork ui_patterns-3589206

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

pdureau created an issue. See original summary.

pdureau’s picture

pdureau commented
Issue tags: +Novice

We can get inspiration from #3592808: Update getSortedDefinitions and getGroupedDefinitions signatures

It works for 11.3, 11.4, and 12. We need to test for Drupal 10.

For information, Drupal 10 compatibility will be dropped in UI Patterns 2.1. See  #3471454: Drop Drupal 10 compatibility

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: Unassigned » grimreaper
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » pdureau

As the ComponentPluginManager is using CategorizingPluginManagerTrait without overriding the getSortedDefinitions and getGroupedDefinitions methods, I think there is nothing to do.

Or have you encountered a bug already in D12?

PS: Other commits

https://git.drupalcode.org/project/drupal/-/commit/13d18b6bd928ec64c0119...
https://git.drupalcode.org/project/drupal/-/commit/3c7b7654433706a36c5ff...

mildertechnologies made their first commit to this issue’s fork.

mildertechnologies’s picture

mildertechnologies commented

Hi @pdureau and @grimreaper,

I am completely new to contributing to Drupal and looking for an issue to work on! I saw this was tagged as a "Novice" issue, which is exactly what I am looking for.

However, I noticed the recent discussion about whether the ComponentPluginManage is already handling this and if there is actually anything left to do for Drupal 12. Before I try to dive in, could you clarify if this issue still needs work?

If it does, @pdureau mentioned earlier that we need to test the implementation for Drupal 10. Since I am a total newbie, would you be able to provide a few pointers on how I can test this locally, or point me toward the specific files I should look at? I am very eager to learn and would love to help out if this task is still active!

Thanks!

pdureau’s picture

pdureau commented
Assigned: pdureau » Unassigned

Hi Sumit,

Thanks for reaching out and welcome to the contrib community. Looking for "Novice" tag is a good move. It is even better to pick unassigned ticket to be sure nobody is currently working on it. This one ticket was assigned to me, but I am happy unassign me so you can have a try.

However, I noticed the recent discussion about whether the ComponentPluginManager is already handling this and if there is actually anything left to do for Drupal 12. If it does, @pdureau mentioned earlier that we need to test the implementation for Drupal 10.

Indeed, this ticket must first be tested with this branch:

By simplify running the ComponentPluginManager, so drush cr may be enough. But it is even better to install ui_suite_library and to go to /admin/appearance/ui/components to try (you need to have at least one SDC available, Core's claro theme provides one).

If it works well, there is nothing to do and you can move the ticket to "Needs review" status and unassigned you.

If KO, you can document it here, move the ticket to "Needs work" status and keep it assigned to you. We will decide a change and test it on 4 different Drupal branches:

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: Unassigned » grimreaper

grimreaper opened merge request !519

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented

As the CategorizingPluginManagerTrait is already providing methods with the $label_key arguments and as UIP component plugin manager does not implement by itself the CategorizingPluginManagerInterface, there is nothing to do.

I have added a comment for Drupal <11.2 because I was tempted to remove the use CategorizingPluginManagerTrait; but it is not present before Core 11.2.0 so still needed for those versions.

  • grimreaper committed 2a64ad33 on 2.0.x 
    chore: #3589206 Drupal 12 and CategorizingPluginManagerInterface

By:...
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » Unassigned
Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.