Choosing custom reader theme does not show its info when said info is no longer supplied

[pierlon]

Bug Description

To summarize the overly complex title, if I activate a plugin that adds a custom reader theme like so:

add_filter( 'amp_reader_themes', function ( $reader_themes ) {
	$reader_themes[] = [
		'name' => 'Neve',
		'slug' => 'neve',
		'preview_url' => 'https://wp-themes.com/neve',
		'screenshot_url' => 'https://i0.wp.com/themes.svn.wordpress.org/neve/2.7.5/screenshot.png?w=1144&strip=all',
		'homepage' => 'https://themeisle.com/themes/neve/',
		'description' => 'This is a non-core theme.',
	];
	return $reader_themes;
} );

Then deactivate said plugin, the following can be seen in the Reader themes section on the Settings page:

Notice that the theme name is not being shown. The reasoning for this makes sense, but as a user I would like to think that the currently activated theme information would persist since the theme is currently installed and active as the Reader theme.

Expected Behaviour

The theme name and details should still be retained if it's the currently selected Reader mode theme.

Steps to reproduce

1. Create and activate a plugin with the above filter
2. Choose the Neve theme as the Reader theme
3. Deactivate said plugin
4. See that the name of the theme is not shown in the Reader themes section, and is also not shown in the themes list

Additional context

• WordPress version:
• Plugin version:
• Gutenberg plugin version (if applicable):
• AMP plugin template mode:
• PHP version:
• OS:
• Browser: [e.g. chrome, safari]
• Device: [e.g. iPhone6]

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

[westonruter]

I suppose we should force the current reader theme to be among the list. If the active reader theme is not in the list after filtering, then we should ensure it gets included.

[jwold]

Here's an updated SVG

Group 3320.svg.zip

[westonruter]

@jwold We need the two desktop and mobile illustrations separated out into separate images, without the Desktop/Mobile labels.

[westonruter]

Here's what I propose:

1. As we discussed earlier, move move the invocation for ReaderThemes::get_classic_mode() to append after the amp_reader_themes are filtered. Aside: We need to translate 'AMP Legacy' here.
2. If the filtered output does not include the currently-selected reader_theme, then obtain the necessary Reader theme data from wp_get_theme( $reader_theme ) and unshift it onto the list of Reader themes.
3. The data obtained from wp_get_theme() will not include a screenshot_url so we should default to showing the screenshot.png from the theme, even though it will be a desktop screenshot. Set the background color of the img to be black and add object-fit:contain so that it is letter-boxed top and bottom.
4. If there is not even a screenshot.png then use the placeholder SVG for the mobile screenshot (and the desktop screenshot on the “Review” step).

[jwold]

@westonruter here you go, happy to make any changes if needed!

Archive.zip

[westonruter]

@jwold Please remove the device frame. So it should be just these rects:

[jwold]

Icons2.zip

Here you go! @westonruter

[westonruter]

I just realized an important implication for this. A theme can now add an amp_reader_themes filter to allow another theme to be used for Reader mode, and that theme can be used for Reader mode even when serving the Reader theme.

[johnwatkins0]

This is passing QA.

1. Set twentytwenty as my reader theme
2. Deleted twentywenty via the command line
3. Refreshed AMP settings. My reader theme was the legacy theme and I saw this notice: