Problem/Motivation

We are in the process of decoupling themes from Classy: #3109287: Decouple Classy-inheriting themes from Classy's preprocess functions

Part of this process is providing each Classy-inheriting theme with the functionality provided by classy.theme, so functionality does not change if Classy is removed. #3109287: Decouple Classy-inheriting themes from Classy's preprocess functions. In that issue, it was noted that this results in quite a bit of repetition that is potentially unnecessary and could be difficult to maintain. @dww mentions:

It's kinda a shame to duplicate this code in all these places, but that's required given a) by our own policy, we can't add these classes in core itself and require themes to do so themselves and b) are moving away from having a shared base theme for core themes. Therefore, there's no good place to put shared code anymore, so we have to live with copy/paste and repeating ourselves. If all core themes need/want to add these classes to the media library, I wonder if a) is actually true. Maybe the better patch would be adding the classes in the media library itself

All three functions in classy.theme are specific to media library. One of these (classy_preprocess_image_widget()) enforces access permissions to see image previews -- that probably shouldn't be tied to a theme anyway. The other two are for adding classes to elements (classy_preprocess_links__media_library_menu(), classy_form_alter()) media_library.module already does it's share of this so perhaps the addition of two more classes is acceptable?

Proposed resolution

Revisit the three functions in classy.theme and determine if it would be acceptable for any of them to be moved to media_library.module. For those that are, move them there.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comments

bnjmnm created an issue. See original summary.

dww’s picture

dww
we/he/they
commented
Issue summary: View changes
Issue tags: +Needs frontend framework manager review

Yes, please! ;)

I think this is mostly up to @lauriii to decide, right? Tagging for review.

Also, fixed bug in the summary that mentioned claro.theme instead of classy.theme.

Thanks for opening this! Glad I'm not alone in thinking this is the better solution. :)

Cheers,
-Derek

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

The current architecture for the distribution of HTML classes was decided in a discussion that happened in 2014: #2289511: [meta] Results of Drupalcon Austin's Consensus Banana. If we think that the current architecture isn't working, we should reopen the discussion.

However, in my opinion, the root cause to the problem in this case is #2195695: Admin UIs on the front-end are difficult to theme. If we were able to solve that issue, we could likely move these preprocess functions to just Seven and Claro. After Claro is stable, Seven will be deprecated for removal and we would then have to maintain only a one instance of these preprocess functions.

IIRC classy_preprocess_image_widget() is working around the problem that {% if data.preview %} would be true even when the access is false. The issue for that problem is: #953034: [meta] Themes improperly check renderable arrays when determining visibility. We might be able to move that to a module since it doesn't have much to do with the markup itself.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented

After reviewing #2289511: [meta] Results of Drupalcon Austin's Consensus Banana, it's apparent that the current approach was not something that was implemented casually. I do think this could be rethought due to the emergence of opinionated core modules such as Media Library and Layout Builder, but the time required for such a change would not be worth postponing #3109287: Decouple Classy-inheriting themes from Classy's preprocess functions, and this could also be accomplished by #2195695: Admin UIs on the front-end are difficult to theme, which is already kinda in motion.

I did some additional investigation on classy_preprocess_image_widget() and it does look like that can be moved to a module.

This surfaced the information I needed to comfortably proceed with #3109287: Decouple Classy-inheriting themes from Classy's preprocess functions. As much as I'd like to move those preprocess functions to the media module, it seems best to focus capacity on #2195695: Admin UIs on the front-end are difficult to theme.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.