Skip to content

OEL-4511: Update to Drupal 11.3.x. #541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tibi2303 wants to merge 1 commit into
base: 1.x
Choose a base branch
from
+53 −8
Open

OEL-4511: Update to Drupal 11.3.x. #541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions oe_bootstrap_theme.theme
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
declare(strict_types=1);

use Drupal\Core\Template\Attribute;
use Drupal\oe_bootstrap_theme\DrupalCompatibility;

// Include all files from the includes directory.
$includes_path = __DIR__ . '/includes/*.inc';
Expand All @@ -21,13 +22,13 @@ foreach (glob($includes_path) as $filename) {
* Implements hook_theme_suggestions_HOOK_alter() for table.
*/
function oe_bootstrap_theme_theme_suggestions_table_alter(array &$suggestions, array $variables) {
if (!theme_get_setting('bootstrap_tables.enable')) {
if (!DrupalCompatibility::themeGetSetting('bootstrap_tables.enable')) {
return;
}

$extra_suggestions[] = 'table__bootstrap';

$responsive = theme_get_setting('bootstrap_tables.responsive');
$responsive = DrupalCompatibility::themeGetSetting('bootstrap_tables.responsive');
if (is_string($responsive) && $responsive !== '') {
$extra_suggestions[] = 'table__bootstrap__responsive';
}
Expand All @@ -41,13 +42,13 @@ function oe_bootstrap_theme_theme_suggestions_table_alter(array &$suggestions, a
* Implements hook_theme_suggestions_HOOK_alter() for views-view-table.
*/
function oe_bootstrap_theme_theme_suggestions_views_view_table_alter(array &$suggestions, array $variables) {
if (!theme_get_setting('bootstrap_tables.enable')) {
if (!DrupalCompatibility::themeGetSetting('bootstrap_tables.enable')) {
return;
}

$extra_suggestions[] = 'views_view_table__bootstrap';

$responsive = theme_get_setting('bootstrap_tables.responsive');
$responsive = DrupalCompatibility::themeGetSetting('bootstrap_tables.responsive');
if (is_string($responsive) && $responsive !== '') {
$extra_suggestions[] = 'views_view_table__bootstrap__responsive';
}
Expand Down Expand Up @@ -121,7 +122,7 @@ function oe_bootstrap_theme_preprocess_views_view_table__bootstrap__responsive(&
*/
function _oe_bootstrap_theme_bootstrap_responsive_table(array &$variables): void {
$class = 'table-responsive';
$breakpoint = theme_get_setting('bootstrap_tables.responsive');
$breakpoint = DrupalCompatibility::themeGetSetting('bootstrap_tables.responsive');
if ($breakpoint !== 'always') {
$class .= '-' . $breakpoint;
}
Expand Down
1 change: 1 addition & 0 deletions runner.yml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ toolkit:
- ~10.5.0
- ~10.6.0
- ~11.2.0
- ~11.3.0
PUBLISH: false
BEHAT: false
PHPUNIT: true
Expand Down
2 changes: 1 addition & 1 deletion src/BackwardCompatibility.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ final class BackwardCompatibility {
* @SuppressWarnings(PHPMD.BooleanGetMethodName)
*/
public static function getSetting(string $name): bool {
return theme_get_setting(self::PREFIX . $name) ?? TRUE;
return (bool) (DrupalCompatibility::themeGetSetting(self::PREFIX . $name) ?? TRUE);
}

}
42 changes: 42 additions & 0 deletions src/DrupalCompatibility.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php

declare(strict_types=1);

namespace Drupal\oe_bootstrap_theme;

use Drupal\Component\Utility\DeprecationHelper;

/**
* Handles Drupal core compatibility behaviors.
*/
final class DrupalCompatibility {

/**
* Service id for the Drupal 11.3+ theme settings provider.
*/
private const THEME_SETTINGS_PROVIDER_SERVICE = 'Drupal\\Core\\Extension\\ThemeSettingsProvider';
Copy link
Contributor

@donquixote donquixote Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need for this constant.
Also we should import the class and use ThemeSettingsProvider::class.
Actually the earlier version was fine, where we had \Drupal::service(ThemeSettingsProvider::class).
Or was there a problem?


/**
* Core version where ThemeSettingsProvider::getSetting() should be used.
*/
private const THEME_SETTINGS_PROVIDER_VERSION = '11.3.0';
Copy link
Contributor

@donquixote donquixote Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a value of having this in a constant.
It makes much more sense to have all the info related to ThemeSettingsProvider / theme_get_settings() centralized in the method.


/**
* Gets a theme setting across supported Drupal core versions.
*
* @param string $setting
* The setting name.
*
* @return mixed
* The setting value.
*/
public static function themeGetSetting(string $setting): mixed {
return DeprecationHelper::backwardsCompatibleCall(
\Drupal::VERSION,
self::THEME_SETTINGS_PROVIDER_VERSION,
fn () => \Drupal::service(self::THEME_SETTINGS_PROVIDER_SERVICE)->getSetting($setting),
fn () => theme_get_setting($setting),
);
}

}
5 changes: 3 additions & 2 deletions theme-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

use Drupal\Core\Form\FormStateInterface;
use Drupal\oe_bootstrap_theme\BackwardCompatibility;
use Drupal\oe_bootstrap_theme\DrupalCompatibility;

/**
* Implements hook_form_system_theme_settings_alter().
Expand All @@ -25,7 +26,7 @@ function oe_bootstrap_theme_form_system_theme_settings_alter(&$form, FormStateIn
'#type' => 'checkbox',
'#title' => t('Style all tables using Bootstrap.'),
'#description' => t('Applies Bootstrap classes to all tables rendered using this theme. Note that some table instances (such as calendars) might not render correctly when Bootstrap is applied to them.'),
'#default_value' => theme_get_setting('bootstrap_tables.enable'),
Copy link
Contributor

@donquixote donquixote Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry for my dead-end and wrong advice earlier.
I did some testing and find that DrupalCompatibility::themeGetSetting($config_name) (without another argument) should just work here.

I tried this with both 11.3 and 11.2.

The reason this works is that ThemeSettingsForm temporarily sets the active theme to whichever theme we are editing, before calling the alter hook. This way, any call to theme_get_setting() or its Drupal 11.3 equivalent will get the value from the theme being edited.

I also found that olivero_form_system_theme_settings_alter() is doing it wrong.
When you enable a sub-theme of olivero, and edit the settings, they are saved to olivero rather than the sub-theme.

Copy link
Contributor

@donquixote donquixote Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another interesting fact: ThemeSettingsForm extends ConfigFormBase, which has its own mechanism to read form values and write them to config on submit. To do that, it looks at the #config_target keys of form elements.

ThemeSettingsForm calls the parent submit method, but that is completely pointless, because generally none of the elements have #config_target.
Instead, it then does its own mechanism to write values to config, grabbing all form values instead of just those where #config_target was set, and writing to the correct theme.

Olivero sets #config_target to e.g. olivero.settings:site_branding_bg_color, which is wrong.
As a consequence, all values are loaded from olivero, then saved to olivero on submit, then saved to the sub-theme (I think).
It is ok because olivero is not designed as a base theme.

'#default_value' => DrupalCompatibility::themeGetSetting('bootstrap_tables.enable'),
];

$form['bootstrap_tables']['responsive'] = [
Expand All @@ -41,7 +42,7 @@ function oe_bootstrap_theme_form_system_theme_settings_alter(&$form, FormStateIn
'xxl' => t('Extra extra large'),
],
'#empty_option' => t('Never'),
'#default_value' => theme_get_setting('bootstrap_tables.responsive') ?? '',
'#default_value' => DrupalCompatibility::themeGetSetting('bootstrap_tables.responsive') ?? '',
];

$form['backward_compatibility'] = [
Expand Down