Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
The buttons on update.php hide their text when Olivero is used as a frontend theme.
Steps to reproduce
- Install standard
- Add the following code to user.install
/** * Tests olivero. */ function user_update_9302() { }
- Login as user 1
- Visit /update.php
Screenshots
The screenshot above shows the Apply pending updates button text completely hidden.
The screenshot above shows the Apply pending updates button with the mouseover state. The button is still partially covered in dark blue.
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
On update.php, $settings['maintenance_theme'] = FALSE
now defaults to Claro or Seven if one of them is installed instead of the default theme. If Claro and Seven are not installed, default theme is still used.
Comment | File | Size | Author |
---|---|---|---|
#15 | 3279640-10.0.x-15.patch | 4.01 KB | Spokje |
| |||
#15 | interdiff_12-15.txt | 1.18 KB | Spokje |
#14 | interdiff_9.4.x_12-13.txt | 1.02 KB | Spokje |
#14 | 3279640-9.4.x-13.patch | 4.34 KB | Spokje |
#10 | Notification_Center.png | 58.46 KB | mherchel |
Comments
Comment #2
alexpottComment #3
alexpottComment #4
alexpottThis swaps the theme to claro instead of seven (which is not installed so we fallback to olivero) for update.php when claro is installed. We still should fix the buttons in olivero for update.php and we also need to fix claro in update.php too... it does not look much better. See the screenshot below:
I toyed with replicating the logic in _drupal_maintenance_theme() to fake install seven so we can fallback to something we know looks good. But after discussion with @lauriii and @catch I'm pretty convinced that fake enabling a theme during the update process is going to lead to more problems than it fixes.
I think we should do the minimal theme work here to make claro and olivero look okay during update.php and fill follow-up issues to refine and make them look great.
Comment #5
lauriiiComment #6
alexpottHere's a small tweak to the current Claro maintenance css that makes it behave more like seven for now. I think we could do something like this as a minimum to unblock release and then refine further in issues like #3085219: Installer is not very usable in Claro.
This patch results in update.php looking like this:
Comment #7
alexpottI think this code allows maintenance_theme to be set to FALSE. And then it'll use your frontend theme. It seems really odd functionality. And it is completely undocumented and not supported by
_drupal_maintenance_theme()
. I think this bit is odd copy and paste from that method where it does:But that only occurs when not in the installer or updating. And when we are we do
$custom_theme = Settings::get('maintenance_theme', 'seven');
and don't do any check for falseness.Comment #8
lauriiiComment #9
SpokjeComment #10
mherchelI tested the patch above in Chrome, Safari, and FF and at multiple viewports. It seems to work perfectly, and the CSS looks correct. I looked over the changes to DbUpdateNegotiator.php, and while I'm not familiar enough with the internals of Drupal's PHP to properly evaluate the code, it seems to be correct, and does the job properly.
RTBC pending tests passing!
Comment #11
mherchelOpened up followup for Olivero at #3279693: Olivero: Hyperlinks with "button" or "button--primary" do not have proper styling when nested in a "text-content" container
Comment #13
alexpottWhoops forgot to inject the service. At least we've proved the deprecation works :)
Comment #14
SpokjeNow with extra valid (unpublished) CR-link.
Comment #15
SpokjeComment #16
SpokjeAnd a
10.0.x-dev
patch without deprecation.Comment #17
lauriiiComment #18
lauriiiComment #23
lauriiiDiscussed with @catch and he asked for a release note on this. I added a draft release note to the IS.
Committed 0f3e14a and pushed to 10.0.x. Committed patch from #14 to 9.5.x and cherry-picked to 9.4.x. Thanks!
Comment #24
catchRelease note looks good - just in case someone gets confused by the theme change.
Comment #26
Gábor HojtsyReviewing this for the release notes of 9.4.0-beta1. The release note snippet says the default theme will still be used if neither Seven nor Claro are installed, but that is not what's in the code? It appears it would fall back on Seven regardless of installation status for Seven.
Comment #27
lauriiiThe code is very confusing 😅 If you check
\Drupal\Core\Theme\ThemeNegotiator::determineActiveTheme
you can see that if the theme is not installed,\Drupal\system\Theme\DbUpdateNegotiator
doesn't do anything.Comment #28
quietone CreditAttribution: quietone at PreviousNext commentedHmm, lost my previous comment.
I added a CR for reference in the release notes, changing to NR for review of those CR.
Comment #29
xjmThanks @quietone!