Problem/Motivation

Themes can now be OOP.
Let's convert Olivero.

Steps to reproduce

Open olivero.theme

Proposed resolution

Use rector to convert

Remaining tasks

Review
Create follow up for organization and DI
Create follow up to replace the remaining functions
_olivero_hex_to_hsl

User interface changes

N/A

Introduced terminology

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

CommentFileSizeAuthor
#12 Screenshot 2026-03-15 at 7.58.41 PM.png221.46 KBsivaji_ganesh_jojodae

Issue fork drupal-3575583

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

nicxvan created an issue. See original summary.

nicxvan opened merge request !14903

nicxvan’s picture

nicxvan commented
Issue summary: View changes
nicxvan’s picture

nicxvan commented
Status: Active » Needs review
smustgrave’s picture

smustgrave commented

Assuming the DI will be done in a follow up?

If so this LGTM.

nicxvan’s picture

nicxvan commented

Yes, DI is a follow up.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

In that case, applying hte MR on a standard install olivero is still rendering fine for me.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

There are 10 calls to \Drupal::, including several repeating ones. I think DI would be doable in scope, it's pretty straightforward with autowire and I don't think it would make the MR significantly harder to review. I think we should even consider splitting this into 2-3 different classes, such as all the entity stuff with the entity type manager into one and the rest in a general one? We already have OliveroPagePreprocessHooks from the initial issue as an example.

Leaving at RTBC, just wanted to add my opinion :)

nicxvan’s picture

nicxvan commented

I really would prefer not to do DI and follow the pattern we did for module hook conversion. The more manual work I need to apply after running rector the more risky this conversion gets.

As it stands it's basically rector plus codesniffing and phpstan.

nicxvan’s picture

nicxvan commented
Issue summary: View changes
nicxvan’s picture

nicxvan commented
Issue summary: View changes
sivaji_ganesh_jojodae’s picture

sivaji_ganesh_jojodae commented
StatusFileSize
new Screenshot 2026-03-15 at 7.58.41 PM.png221.46 KB

A follow-up issue should be created to add type hints for all variables.

nicxvan’s picture

nicxvan commented
Related issues: +#3493453: [meta] Standardize and clean up hook classes in core

A post conversion cleanup issue already exists: #3493453: [meta] Standardize and clean up hook classes in core.

I've linked it here.

  • catch committed 56c6081d on main 
    task: #3575583 Convert Olivero to OOP

By: nicxvan
catch’s picture

catch
he/him
Primary language English
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

I think the 2/3 phase approach is still good here. It's easier to be confident about the rector conversion, then splitting etc. individually than all that once.

Committed/pushed to main, thanks!

I wasn't able to cherry-pick this to 11.x, looks like it was accidentally backported already as part of c03369417f900ee7bbd3bc1235ca304dc7ea8955 We could revert that commit, commit the intended bits, then backport this, if we want more explicit git history, or leave things as is. Leaving to be ported to figure out.

catch closed merge request !14903

  • catch committed 7fc11165 on 11.x 
    task: #3575583 Convert Olivero to OOP

By: nicxvan
(cherry picked from...

  • catch committed 6b1671d0 on 11.x 
    Issue #3575583: revert accidental backport
catch’s picture

catch
he/him
Primary language English
commented
Status: Patch (to be ported) » Fixed

I reverted only the mistaken big of the 11.x commit courtesy of patch -p1 -R and then did the actual cherry-pick. Should be OK now.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

nicxvan’s picture

nicxvan commented
Assigned: nicxvan » Unassigned
sivaji_ganesh_jojodae’s picture

sivaji_ganesh_jojodae commented

Following #12, I've created a related issue at https://www.drupal.org/project/drupal/issues/3579922

Status: Fixed » Closed (fixed)

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