Problem/Motivation

There are usages for forum CSS in themes/profiles that need to be removed so the module can be removed.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3409384

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

quietone created an issue. See original summary.

quietone’s picture

quietone commented
Title: Remove forum from themse » Remove forum from themes

Fix typo in title

smustgrave’s picture

smustgrave commented
Assigned: Unassigned » smustgrave

smustgrave opened merge request !6154

smustgrave’s picture

smustgrave commented
Issue summary: View changes

Just moved claro but let me know if that works.

kostyashupenko made their first commit to this issue’s fork.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs work
andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review

It looks good to go!

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Still need to move olivero and stable9. But if we agree on the approach should be simple.

Background on the changes I did for hook_theme() was because not all themes had their own full set of forum templates. Claro actually had 4/5.

andypost’s picture

andypost
he/him
Primary language Russian
commented

I find better to keep the same name for libs - less to add to change record

also it needs empty update hook to rebuild registry as files are moved and no need to apply https://www.drupal.org/about/core/policies/core-change-policies/drupal-d...

smustgrave’s picture

smustgrave commented

Wasn't 100% for that since it was classy code in claro but makes sense to me.

quietone’s picture

quietone commented

I made the same modifications to the olivero and stable9. If that is the right thing to do then that bulk work is done. However, there are failing tests. Leaving at NW.

smustgrave’s picture

smustgrave commented

Moved some more conditions to the hook_theme and updated the css path for olivero.

@Wim Leers did post one question on the Tour related ticket for hook_library_info_alter what should happen if there was a base theme to stable9?

quietone credited catch.

quietone credited larowlan.

quietone credited lauriii.

quietone’s picture

quietone commented
Status: Needs work » Postponed
Parent issue: #3267261: [Meta] Tasks to deprecate Forum » #3274153: [11.x] [Meta] Tasks to remove Forum from core and move to contrib

This was discussed in #need-review-queue-initiative in Slack, https://drupal.slack.com/archives/C04CHUX484T/p1708034907274609.

The conclusion was that this is best done in Drupal 11 when removing the extension from core. This applies to Book, Forum and Tour.

@catch @quietone @wimleers (he/him) so not sure what’s needed to move forward on #3405660: Remove tour from themes any suggestion would be appreciated

larowlan I wonder if we can just delete this code?
larowlan Core themes don't have code for {random contrib modules}
larowlan it feels odd that [random contrib module] has special case for [some core theme]
larowlan where contrib module is tour in this case
larowlan where does it stop, does tour start having to add styling for random contrib themes too
larowlan feels like it could be a separate contrib module - e.g. we have lb_claro that provides LB styling for claro
smustgrave And this is tour but also probably a blocker for forum too
larowlan Could be sub module that people can enable if they want?
smustgrave Not sure the correct answer. I probably was going to consolidate the css into one file once this is in contrib
smustgrave And drop the rest.
quietone Sadly, I can't help with what to do about the theme code itself.
quietone I will be doing admin on the extension removals today. Particularly about ensure that the contrib versions are ready and the policy implementation docs need some adjustments.
smustgrave This should be the last one for tour, hopefully
quietone Book and Tour also have open issues about removing themes.
smustgrave Think all deprecated modules with css are sorta blocked till it’s decided what to do with them
smustgrave May extend to any deprecated module that has css or a custom theme template actually
quietone In the book issue lauriii suggested using hook_theme_registry_alter
larowlan cc @lauriii
quietone I wonder if we can just delete this code?Ah, what code exactly? You mean not even moving the css files to tour, book, forum?
quietone If may feel odd to special case a core theme but the extension should retain the same functionality when moved to contrib.
larowlan yeah I'm advocating removing it and putting it in a submodule of the removed module
larowlan 'tour_claro' or 'tour_stable'
larowlan but agree with stephen, once it lands in contrib I'll likely either move it into forum.css or drop it
catch I'm not sure new modules will work in core, we'd need an upgrade path to enable them (or decide not to), they 'd show up in the UI etc.
What about if we leave the libraries in the themes in Drupal 10, and open an 11.x-only issue to remove it. And the contrib module is responsible for doing whatever it needs to do to look right.
catch As long as nothing breaks when the modules are deprecated that's not really doing any harm.
lauriii That works for me. Would be much simpler too.
smustgrave So close this ticket? But when this moves to contrib it’s not broken styling wise on core least for that first release is that okay?
catch We should double check that deprecating one of the modules with this issue doesn't cause any test failures, I don't think it will because it's all soft dependencies but worth checking.
smustgrave I think that’s the last ticket in the meta is to officially deprecate.  Unless you mean something else? First time following a deprecated module fully through
catch I mean we should add an MR to actuall deprecate tour or forum, and make sure that umami/stable9 etc. don't cause test failures with the supporting CSS/library definitions still there. If they don't, then leaving the CSS in Drupal 10 should work.
catch And if it's fine, that's the next step anyway so we can just go from there.
smustgrave https://git.drupalcode.org/project/drupal/-/merge_requests/6648 all the failures seem to be coming from tour itself (edited)
catch Nice it probably just needs @group legacy on a load of tests hopefully. I think we should do it this way then.
smustgrave So got that MR all green, should I close the tour theme ticket?
catch Maybe repurpose for the 11.x-only work?
smustgrave You mean the removal?
catch Yeah removal of the CSS from claro et al.
catch The module removal can happen after that 'cos it'll need update tests adjusting and other painful things so two issues is easier.
smustgrave Ok so I unpostponed #3405660: Remove tour from themes
smustgrave Created the CR, have the contrib space, but not sure who updates https://www.drupal.org/node/3223395
quietone And does this also apply to remove themes from Book and from Forum? #3409384: [11.x] Remove forum CSS from themes and profiles
larowlan I think it does, as its the same outcome we're after

Participants:

larowlan, smustgrave, quietone, catch, lauriii

Adding credit and changing parent.

quietone’s picture

quietone commented
Parent issue: #3274153: [11.x] [Meta] Tasks to remove Forum from core and move to contrib » #3261653: Remove Forum module

Spokje made their first commit to this issue’s fork.

Spokje changed the visibility of the branch 11.x to hidden.

Spokje opened merge request !6754

Spokje changed the visibility of the branch 3409384-remove-forum-from to hidden.

spokje’s picture

spokje
Primary language Dutch
commented

Out of curiosity and to see if tests would break if we only deleted CSS I opened a new MR.

Deleted forum CSS from themes claro, olivero and stable9 and also from profile umami.
Tests are OK.

Am a bit unsure if this is still postponed now that 10.3.x is opened.
Can we diverge in 11.x for 11.0.x already?

catch’s picture

catch
he/him
Primary language English
commented

Can we diverge in 11.x for 11.0.x already?

Yes!

spokje’s picture

spokje
Primary language Dutch
commented
Title: Remove forum from themes » Remove forum CSS from themes and profiles
Issue summary: View changes
Status: Postponed » Needs review

Woohoo, thanks @catch!

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Searched themes directory for forum and all css has been removed.

spokje’s picture

spokje
Primary language Dutch
commented

Found and removed 2 stragglers (forum-icons.png)

spokje’s picture

spokje
Primary language Dutch
commented
Title: Remove forum CSS from themes and profiles » [11.x] Remove forum CSS from themes and profiles
quietone’s picture

quietone commented
Status: Reviewed & tested by the community » Closed (duplicate)

I moved these changes into the parent issue so we remove everything at once. I have moved credit over there. I am not sure what the best status is but duplicate seems the best.

smustgrave closed merge request !6154

quietone’s picture

quietone commented
Version: 11.x-dev » 11.0.x-dev

Update version to the branch this applies to.