-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove Classy and update to use starterkit #61
Comments
@occupant should I create the PR against |
Did a quick test locally and generally it was very simple.
Subtheme issues
|
@joelpittet Yes, I think master, at least for now, but we could use the opportunity to pull in some things from develop |
@occupant a bit of scope creep but the command you ran makes a theme based on I plan on doing based on |
@joelpittet Oh, maybe I went about this wrong. I generated a new theme called Galactus first using 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 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. |
@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 😅 |
@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
|
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 |
@joelpittet As per message, Classy doesn't currently support being used as a starterkit (missing the required I was able to get around this by adding it manually to Classy. I could then generate a new theme with I also, by way of comparison, generated a new theme using the defaults via 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. |
@joelpittet I've taken an initial stab at it here: |
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
Setting |
I think a version is fine to add back, we should really try to version it in releases anyway I suppose |
I hope to spend some time pouring over the diff later today, thanks @occupant |
Made a note on ubc-web-services/clf#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). |
@occupant We are seeing what you reported in #61 (comment) |
#67 Fixes lots of the issues we ran into. |
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
The text was updated successfully, but these errors were encountered: