ubc-web-services / galactus

A Drupal version of the UBC CLF theme
MIT License
6 stars 5 forks source link

Remove Classy and update to use starterkit #61

Open occupant opened 1 year ago

occupant commented 1 year ago

As per discussions with Joel, Darko, Classy is now deprecated, so we should move to use starterkit instead.

https://www.drupal.org/docs/core-modules-and-themes/core-themes/starterkit-theme

joelpittet commented 1 year ago

@occupant should I create the PR against master or develop? I'm assuming master

occupant commented 1 year ago

Did a quick test locally and generally it was very simple.

  1. The classy/base library is excluded from Galactus, so this library and assets can be safely removed, but should we further investigate?
  2. When generating a new sub theme via php core/scripts/drupal generate-theme --starterkit galactus my_new_theme, there are several issues. Is this something we need to overcome?

Subtheme issues

  1. config namespace not changed (not unexpectedly)
  2. theme-settings.php function namespace not changed
  3. breakpoints settings key namespace not changed
occupant commented 1 year ago

@joelpittet Yes, I think master, at least for now, but we could use the opportunity to pull in some things from develop

joelpittet commented 1 year ago

@occupant a bit of scope creep but the command you ran makes a theme based on galactus, we could ALSO recommend that and ditch the "UBC" theme example? AKA, no more extending base themes (though need an update path)

I plan on doing based on classy and copying the overrides back over the result in the PR

occupant commented 1 year ago

@joelpittet Oh, maybe I went about this wrong. I generated a new theme called Galactus first using php core/scripts/drupal generate-theme galactus. Then I copied in the Galactus assets (config directory, js, css, images, templates, etc) and updated the .info and template.php to consolidate them.

At that point everything seems to work just fine and we're decoupled from classy.

Looking back at that, I see I should have specified the galactus theme to be based off of classy, as you mentioned.

But next, I wanted to see what would happen if I then created a new theme (not subtheme - I misspoke above) based off of the updated Galactus. That's when I ran php core/scripts/drupal generate-theme --starterkit galactus galactusjr The subtheme [sic] issues are in galactusjr at that point.

Creating galactusjr was more a test to head off the inevitable request / question (new theme vs sub theme). But there do seem to be issues with that and I'm not sure if they're worth being concerned about.

joelpittet commented 1 year ago

@occupant I created the fork, not sure the namespace issues you mentioned above but maybe I should just do it too (for the experience) and then it might be obvious. I'll do that this afternoon (strike while the iron's hot) https://xkcd.com/356/

CC @darkodevubc In case you didn't/don't see this thread from your email already 😅

occupant commented 1 year ago

@joelpittet Some more specifics about the issue I see when using the starterkit generated galactus theme to then generate an additional theme (galactusjr)

galactusjr.breakpoints.yml (lines 1, 7, 13, 19, 25, 31)

result

theme-settings.php (line 13)

result

config/install/

filename result

config/schema/

filename result

config/schema/galactus.schema.yml (line 3)

result

config/install/block.block.breadcrumbs.yml (line 9)

result

occupant commented 1 year ago

Of course, the easiest and maybe best way to resolve the issues above are to add a note that Galactus can't be used to generate additional themes and should use a child theme

occupant commented 1 year ago

@joelpittet As per message, Classy doesn't currently support being used as a starterkit (missing the required starterkit: true in the .info file).

I was able to get around this by adding it manually to Classy. I could then generate a new theme with php core/scripts/drupal generate-theme galactus --starterkit=classy.

I also, by way of comparison, generated a new theme using the defaults via php core/scripts/drupal generate-theme galactus (which seems to use stable9 as the starterkit if the base theme key in the .info file is an indication).

There are quite a few differences. I made a quick diff to show the files (left in attached image) from a default generated theme compared to (right in attached image) a theme generated with Classy as the starterkit. The key differences are additional .twig templates in Classy. I've included a screenshot and a diff of the changed files. diff-galactus.txt galactus-stable9-vs-galactus-classy

occupant commented 1 year ago

@joelpittet I've taken an initial stab at it here: https://github.com/ubc-web-services/galactus/compare/master...feature/61-remove-classy-and-update-to-use-starterkit We could remove the galactus/base (from classy/base) library and assets safely, but I've left them there for now

occupant commented 1 year ago

Looks like Galactus should have a version now in the .info. When I try to generate a theme using Galactus as a starterkit (which is allowed with the starterkit: true in the .info), you get a warning:

The source theme galactus does not have a version specified. This makes tracking changes in the source theme difficult. Are you sure you want to continue? (yes/no) [yes]:

Setting starterkit: false prevents Galactus from being used as a starterkit though and would be an acceptable solution.

joelpittet commented 1 year ago

Looks like Galactus should have a version now in the .info.

I think a version is fine to add back, we should really try to version it in releases anyway I suppose

joelpittet commented 1 year ago

I've taken an initial stab at it here: master...feature/61-remove-classy-and-update-to-use-starterkit We could remove the galactus/base (from classy/base) library and assets safely, but I've left them there for now

I hope to spend some time pouring over the diff later today, thanks @occupant

darko-hrgovic commented 1 year ago

Made a note on https://github.com/ubc-web-services/clf/issues/27 that Galactus will be changing to starterkit and pondered how that will affect the CLF child theme.

I will also be creating a child theme from the CLF for each of our sites, so they'll likely need to be rebuilt using the CLF as starterkit.

That's a lot of template files stored in each theme (rather than previously inheriting from parent).

joelpittet commented 1 year ago

That's a lot of template files stored in each theme (rather than previously inheriting from parent). @darko-hrgovic Yes, embrace the dark side of the fork!

@occupant We are seeing what you reported in https://github.com/ubc-web-services/galactus/issues/61#issuecomment-1376515106

joelpittet commented 1 year ago

67 Fixes lots of the issues we ran into.