If you install with the Umami demo profile and go to the Appearance page, the Umami theme isn't listed there. This means that if you switch to a different theme, you can never switch back.
This can be fixed simply by removing the hidden: true
from the theme's info file. My understanding is that originally existed to prevent users of other install profiles from seeing the theme, but it isn't necessary now that the theme lives inside the profile (since putting it inside the profile in the codebase automatically prevents sites installed with a different profile from seeing it).
This was intended to be fixed in #2809635-171: Create experimental installation profile, but somehow that code change didn't make it into the final commit on that issue?
Comment | File | Size | Author |
---|---|---|---|
#12 | interdiff-7-12.txt | 534 bytes | markconroy |
#12 | 2938918-12.patch | 1.31 KB | markconroy |
#7 | 2938918-7.patch | 1.32 KB | David_Rothstein |
#7 | 2938918-7-TESTS-ONLY.patch | 890 bytes | David_Rothstein |
#2 | 2938918-2.patch | 458 bytes | David_Rothstein |
Comments
Comment #2
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedHere is a patch.
Comment #3
BerdirHa, was wondering about this one too.
You also had the same thought as I did.. "Hm, this looks nice, lets see what happens if I switch to Bartik... oops." :)
Comment #4
smazApologies, I thought that fix was included in one of the last patches for the main issue - must have gone AWOL at some point.
I've already tested this change before, so marking as RTBC:
https://github.com/lauriii/umami/commit/4e382910b956573c1e93ee463572e23d...
Comment #5
markconroy CreditAttribution: markconroy as a volunteer and at Annertech commentedIn my case, I switched to Bartik when on simplytest me testing the layout builder modules (so I'd have some content to see/work with), then couldn't get back to my beloved Umami.
Nice to see it fixed.
Comment #6
larowlanCan we expand on the existing tests to visit the admin appearance page and assert the umami theme is present.
Slowly...expanding...the test coverage...one...issue...at...a...time
Thanks folks
Comment #7
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedSomething like this?
Comment #8
larowlanAny reason not to use
pageTextContains
?Comment #9
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedSo with the theme displaying on the Appearance page again, that actually exposes a couple other bugs - adding those as related issues now.
Comment #10
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedNot sure - I was basically just roughly copying this from https://api.drupal.org/api/drupal/core%21modules%21system%21src%21Tests%... (which uses a regular expression also).
Comment #12
markconroy CreditAttribution: markconroy as a volunteer and at Annertech commentedNew patch using pageTextContains
(This'll be my first ever commit in a test if it's commited!)
Comment #13
navneet0693 CreditAttribution: navneet0693 as a volunteer and at QED42 commentedThat should be all good.
Comment #14
catchCommitted 6e06ffe and pushed to 8.6.x. Thanks!
Comment #16
smazSetting to needs review just to check something...
I think that we actually need the first patch in #7, and not the one from #12.
The test is for checking that Umami is the default theme ("Tests that the Umami theme is the default theme on the Appearance page."), but using pageTextContains("Umami") just checks that the theme is there, not enabled as default.
The regex in pageTextMatches() is used instead of pageTextContains(), due to the version number of the theme being between the theme name & (default theme) text:
Comment #17
markconroy CreditAttribution: markconroy as a volunteer and at Annertech commented@smaz,
I don't think we need to test if the theme is the default theme or not, just to be able to switch back to it. All we need to do is make sure that the theme is available on the page.
Maybe we could do another test to check if it is the default theme as well (but that would should probably be run at install time - once the site is installed, check that Umami is the default theme).
Comment #18
catchAgreed with #17, however the comment for the test does say it's checking for the default theme, not that the theme is available, which is the bug this patch is fixing. @smaz could you open a follow-up to expand the test coverage and/or fix that comment?
Comment #19
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedCreated a followup issue with a quick patch.