Problem/Motivation

Obsolete theme are displayed at admin/appearance.

Steps to reproduce

  1. Create a custom theme with lifecycle: obsolete
  2. Go to admin/appearance

Proposed resolution

Do not allow display of obsolete themes

Remaining tasks

  1. Patch
  2. Review
  3. Test
  4. Commit

User interface changes

Before:

After:

API changes

  • A new function to check if the extension is obsolete or not were added. See: core/lib/Drupal/Core/Extension/Extension.php::isObsolete

Release notes snippet

  • The obsolete themes will not be displayed anymore on admin/appearance page.
  • A new function to check if the extension is obsolete or not were added.
CommentFileSizeAuthor
#9 3265362-8.patch3.18 KBquietone
#9 interdiff-4-8.txt694 bytesquietone
#4 After.png21.71 KBquietone
#4 before.png24.12 KBquietone
#4 3265362-4.patch3.18 KBquietone
#4 3265362-4-fail.patch2.23 KBquietone
#4 interdiff.txt841 bytesquietone

Comments

quietone created an issue. See original summary.

quietone’s picture

quietone commented
Parent issue: #3258782: Do not display obsolete modules at admin/modules » #3135100: [policy and meta] Provide a proper mechanism for deprecating modules and themes
xjm’s picture

xjm
she/her
Primary language English
commented
Priority: Normal » Major

Good idea!

quietone’s picture

quietone commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new interdiff.txt841 bytes
new 3265362-4-fail.patch2.23 KB
new 3265362-4.patch3.18 KB
new before.png24.12 KB
new After.png21.71 KB

I used from isObsolete from murilohp's work in #3258782: Do not display obsolete modules at admin/modules.

The last submitted patch, 4: 3265362-4-fail.patch, failed testing. View results

murilohp’s picture

murilohp commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

I've applied the patch locally and tested it using the following data on my test.info.yml:

name: Test
type: theme
description: 'A flexible, recolorable theme with many regions and a responsive, mobile-first layout.'
package: Core
version: VERSION
lifecycle: obsolete
lifecycle_link: 'https://example.com/obsolete'
base theme: false
core_version_requirement: ^8|^9

The code looks good, the testbot is happy, I've updated the IS with the API changes(isObsolte function) and wrote a release notes snippet.

Once this lands, I can update #3258782: Do not display obsolete modules at admin/modules, since we're using the same new function for both issues, I think we'll need a reroll.

So for me it's RTBC. Thanks!

dww’s picture

dww
we/he/they
commented 
+++ b/core/modules/system/src/Controller/SystemController.php
@@ -204,6 +204,11 @@ public function themesPage() {
+    $themes = array_filter($themes, function ($module) {
+      return !$module->isObsolete();

Slight nit: s/$module/$theme/

Otherwise, this looks great! I love how small and simple the test changes need to be.

dww’s picture

dww
we/he/they
commented

Started a draft CR for both this and #3258782: Do not display obsolete modules at admin/modules.

quietone’s picture

quietone commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new interdiff-4-8.txt694 bytes
new 3265362-8.patch3.18 KB

@dww, thanks.

dww’s picture

dww
we/he/they
commented
Status: Needs review » Reviewed & tested by the community

Perfect, thanks!

  • catch committed d7a82b5 on 10.0.x 
    Issue #3265362 by quietone, dww, murilohp: Do not display obsolete...

  • catch committed f200a37 on 9.4.x 
    Issue #3265362 by quietone, dww, murilohp: Do not display obsolete...
catch’s picture

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

I'm not entirely sure we have a use case for marking a theme obsolete (rather than deprecating and moving to contrib), but it makes sense to support it consistently regardless.

Committed d7a82b5 and pushed to 10.0.x. Also cherry-picked to 9.4.x. Thanks!

Status: Fixed » Closed (fixed)

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