Remove tour from themes

So I moved the CSS to their own libraries so they should still function with these themes and are still there for when we move tour back.

I've pinged FEFM about this. I'm not sure if this needs to be done at all. I don't think it prevents removal of the module but I'll get them to confirm.

So just for the record did this approach so it will still work with each theme and be easier to re add later on

I'm fairly sure we've done something similar before but I can't remember where. I tried to look at couple potential places but didn't find anything.

I'm removing the tag because I don't think that the issue needs FEFM review given that it's essentially just an approach for the contrib module. I think it's enough that the approach is confirmed by @smustgrave who is the maintainer of the contributed module.

In that case this is RTBC then

Thanks

This needs a rebase for a conflict in claro.libraries.yml

I think it ought to be possible to add the additional files into the claro libraries definitions using hook_library_info_alter() - this would be the module equivalent of libraries extend - the main advantage of that is it runs on library info build rather than every HTML request (and it might mean the files aren't added when the tour libraries aren't there, although if they're every page then that'll be the same anyway). However optional whether we do that since the existing approach successfully encapsulates the tour-specific stuff in the module and should work fine.

So like this?

It might need the files added directly to the end of the css or js entries - assuming that they need to override tour's own CSS, if it's a dependency it'll be loaded before. Didn't test this though and don't know exactly what they're doing though.

Last one for 🌱 [Meta] Tasks to deprecate Tour module Active 🚀

Found two potential regressions in the MR. Eliminated one, leaving just the one — curious what y'all think.

Added a way to check base theme. But not sure we should worry too much about contrib themes here as we are well in advance announcing Tour is deprecated. Contrib themes should update their code to handle that.

That introduced test failures:

Exception: Warning: Undefined property: Drupal\Core\Extension\Extension::$base_theme
tour_library_info_alter()() (Line: 125)

Test failures resolved.

No more remarks 👍

@Wim Leers, thanks!

One and a half questions on the MR, might be me getting confused.

Is this something we need to be concerned about? Shouldn’t it be up to the contrib themes to update?

Oops I didn't realise I'd actually committed this, then it got pushed with another change today, reverted the commit pending #19.

@smustgrave well once this MR is committed, if it breaks in a claro subtheme situation, that would happen before tour moves to contrib, if it's the difference of a couple of lines here I think it's worth checking. Also the logic doesn't look right to me there unless I'm really missing something.

Sounds like we have to loop through all sub themes

Doing a loop to get all the sub-themes

smustgrave → closed merge request !5657

The theme removal was discussed in slack https://drupal.slack.com/archives/C04CHUX484T/p1708034907274609

It was agreed upon that leaving the CSS is fine and when we remove tour we can remove the css from the themes

So going to delete the css and postpone this.

Seems we can merge these now :)

This displeased Drupal\KernelTests\Core\Theme\Stable9LibraryOverrideTest::testStable9LibraryOverrides:
https://git.drupalcode.org/issue/drupal-3405660/-/jobs/891571#L3438

Thought with the css being removed that wouldn't care but guess since the module is still there. Guess this re-postponed :(

This work is being done in 📌 Remove Tour module Postponed . I'll close this as a duplicate and move credit.