Drupal Current theme condition plugin should provide an option to select all themes

Tagging for framework input. If this is a path we want to go down.

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

Think it will help for steps to reproduce this bug.

With a UI change before/after screenshots should be added to the issue summary.

The current theme condition is not shown on blocks because of:
https://git.drupalcode.org/project/drupal/-/blob/10.1.x/core/modules/blo...

    // @todo Allow list of conditions to be configured in
    //   https://www.drupal.org/node/2284687.
    $visibility = $this->entity->getVisibility();
    $definitions = $this->manager->getFilteredDefinitions('block_ui', $form_state->getTemporaryValue('gathered_contexts'), ['block' => $this->entity]);
    foreach ($definitions as $condition_id => $definition) {
      // Don't display the current theme condition.
      if ($condition_id == 'current_theme') {
        continue;
      }

Should we recover this condition?

We are changing the plugin because of https://www.drupal.org/project/google_tag/issues/3263122 ✨ Recovery the current theme condition plugin Active , to be able to load the google tag on any theme, and recover this plugin on the module. Now is excluded because is not possible to select any theme.

Updated to apply to the last drupal version

Added patch also and diff between last commit and the current one, allowing blocks to use the current theme condition.

Added steps to test and captures as asked by @smustgrave

I will review the failed tests https://www.drupal.org/pift-ci-job/2710393 →

Fixed tests, ready to review

I think the test coverage is okay.

But found the workflow a little odd.
I did a Standard install
Went to block layout
Added a block under the Olivero tab but have it only appear on the claro theme.
Tagging for usability for that

Now if we had a global block layout tab then this setting would make sense I think.

Will leave in review for framework manager thoughts.

@smustgrave Do you think the options should be "current theme" or "any" in the case of block layout? instead of the list of all themes.
For the block layout, it have sense because it has theme tabs, for other cases, the list should have all themes

Could someone expand the issue summary to explain why is this setting needed? Drupal Block UI is built so that blocks are intended to be specific to a theme. If we want to change that, it would be helpful to understand why that's desired.

@lauriii You are totally right, I miss the original scope of the issue.

We use current theme condition on google_tag module, but we need the "- any -" option https://www.drupal.org/project/google_tag/issues/3263122 ✨ Recovery the current theme condition plugin Active to have the ability to use google_tag on all themes or only on a theme, for blocks the current_theme condition does not have any sense at all, I will remove the logic that I changed related with blocks and update the summary.

Add MR changes as patch

Seems like the block is the only drupal core entity that uses the "plugin.manager.condition", and the current theme is excluded in this case as explained in previous comments so in the example I will use the google_tag scenario.

Why a select list here when all the other variations of this use checkboxes?

1. +++ b/core/modules/system/src/Plugin/Condition/CurrentThemeCondition.php
   @@ -111,11 +112,18 @@ public function evaluate() {
   +      $summary = $this->isNegated() ?
   +        $this->t('The current theme is not @theme.', ['@theme' => $theme]) : $this->t('The current theme is @theme.', ['@theme' => $theme]);
   +    }
   +    else {
   +      $summary = $this->isNegated() ?
   +        $this->t('The current theme is not one of the enabled themes.') : $this->t('The current theme is one of the enabled themes.');
        }
    
   -    return $this->t('The current theme is @theme', ['@theme' => $this->configuration['theme']]);
   +    return $summary;
   

   we can return early here and avoid using else

2. +++ b/core/tests/Drupal/KernelTests/Core/Plugin/Condition/CurrentThemeConditionTest.php
   @@ -27,12 +27,20 @@ public function testCurrentTheme() {
   +    $this->assertEquals(new FormattableMarkup('The current theme is not one of the enabled themes.', ['@theme' => '']), $condition_any_negated->summary());
   

   This feels like a UX WTF?

   How can a current theme not be one of the enabled themes?

   So as such I think we probably want to prevent the negate option when 'Any' is selected

Nice work folks

#37 works for me on 10.2.2

Just ran into this when working on ✨ Create an admin route condition plugin or add generic "Current theme" options for admin and default theme Active and I think we should rate this as major issue, because currently once you saved the conditions, this condition is always added and evaluated and user has no way to disable this rule?!

One important addition: The form element should be checkboxes, not radio!
It makes no sense to force selecting only one theme!

Selecting no theme at all should always evaluate to TRUE!