Problem/Motivation

This is a follow-up issue to #2783897: Current theme condition plugin should provide an option to match all themes and introduce checkboxes.

The "CurrentThemeCondition" does not have a dependency on the configured allowed theme (and upcoming "themes") config and therefore potentially uninstalled themes can stay in the config.

Steps to reproduce

Proposed resolution

Implement "DependentPluginInterface::calculateDependencies()" for the "CurrentThemeCondition", so that the condition is updated based on the status of the configured theme(s).

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3590591

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

grevil created an issue. See original summary.

grevil’s picture

grevil commented

Postponed on #2783897: Current theme condition plugin should provide an option to match all themes and introduce checkboxes.

grevil’s picture

grevil commented
Related issues: +#2783897: Current theme condition plugin should provide an option to match all themes and introduce checkboxes
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Postponed » Active
Issue tags: +Novice

I think this can be done independently of #2783897: Current theme condition plugin should provide an option to match all themes and introduce checkboxes.
Maybe it's even a nice novice task, looking at existing implementations this should not be too hard, with some assistance?

Please remove the tag if you disagree @core team

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

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

eiki_hanai opened merge request !15871

eiki_hanai’s picture

eiki_hanai commented
Status: Active » Needs review

I have created a Merge Request for this issue.
I implemented DependentPluginInterface::calculateDependencies() for CurrentThemeCondition.
Please review it. Thanks!

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Ran the test-only feature since can't on the MR

The theme dependency should be present when a theme is configured.
Failed asserting that an array has the key 'theme'.
/var/www/html/core/tests/Drupal/KernelTests/Core/Plugin/Condition/CurrentThemeConditionTest.php:71

Shows the coverage and fix seems pretty straight forward. LGTM.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Thanks @smustgrave so hopefully both issues can be merged soon :)

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

I think we should remove the custom assertion messages in the test (see comments on MR), otherwise this looks good.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs work » Needs review

@catch thanks! I removed them. Fine like this?