Right now, when you switch themes from Bluemarine, nothing actually appears to happen. I spent about 30 seconds clicking around before I realized that the new theme _was_ in fact being applied, but only to non-admin pages.

"System default" should always be the setting unless something else was specifically put in to eliminate this confusion.

CommentFileSizeAuthor
#20 patch_42.txt1.2 KBwebernet
#18 themes.patch1.18 KByched
#16 admin_theme_default.patch.txt2.49 KBdavemicc
#15 admin_theme_default_3.patch2.45 KBedmund.kwok
#14 admin_theme_default_2.patch2.49 KBwebchick
#12 admin_theme_default_1.patch2.81 KBwebchick
#11 admin_theme_default_0.patch3.46 KBwebchick
#9 admin_theme_default.patch3.05 KBwebchick
#8 admin-theme_5.patch2.69 KBedmund.kwok
#7 admin-theme_4.patch2.04 KBedmund.kwok
#6 system.admin-theme.patch.txt1.74 KBRobrecht Jacques
#5 admin-theme_3.patch1.54 KBedmund.kwok
#1 admin-theme_2.patch1.75 KBwebchick
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Assigned: Unassigned » webchick
Status: Active » Needs review
FileSize
admin-theme_2.patch1.75 KB

Here's a patch.

Dries’s picture

Dries CreditAttribution: Dries commented

I was confused by this too. Why don't we remove the 'system default' option and just go with the theme names?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

Because the theme names are what cause this problem. :)

By default, that would make the variable "bluemarine" which means the admin theme stays "bluemarine" until you deliberately switch it to "chameleon" or whatever. Then if you switch your theme again (say you're trying out 10 different themes to see what they look like), you have to go back _again_ and switch it to "Theme X."

"System default" makes the admin theme and the "normal" theme match. If you want to override this and make the admin theme always bluemarine, then you can also do that, but this way the default behaviour remains the same.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

Grrr.

I hate to bump my own issue, but this just wasted another 5 minutes of my life because I forgot this wasn't fixed and
couldn't figure out my bloody theme wouldn't change from bluemarine.

Please can we get this fixed?? Pretty please? ;)

edmund.kwok’s picture

edmund.kwok CreditAttribution: edmund.kwok commented
FileSize
admin-theme_3.patch1.54 KB

+1 on the suggestion.

Reduces confusion if users don't go thru all the administrative settings.

I was porting a theme to Drupal for the first time and changed the site theme to the new one. Nothing happened and I checked all the files and everything required was there. When I logout, only then the theme appear and I realized that the administrator theme was set to bluemarine by default.

When default value of admin_theme is set to 0 (integer), pushbutton is selected by default. Changed this by setting '0' as a string and 'System default' is selected by default. Patch works on HEAD.

Code still needs review...

Robrecht Jacques’s picture

Robrecht Jacques CreditAttribution: Robrecht Jacques commented
FileSize
system.admin-theme.patch.txt1.74 KB

I think it makes sense to have admin_theme == system default instead of bluemarine. The (rerolled) patch does that.

On the other hand, would one of the following options also not limit confusion:
- display a message on admin/build/themes when changing the default theme *and* the admin theme is set,
- or, disable the admin theme for admin/build/themes?

Personally, i think that webchicks solution + a message (only if admin_theme != system default) would be best.

edmund.kwok’s picture

edmund.kwok CreditAttribution: edmund.kwok commented
FileSize
admin-theme_4.patch2.04 KB

Perhaps a message like this?

edmund.kwok’s picture

edmund.kwok CreditAttribution: edmund.kwok commented
FileSize
admin-theme_5.patch2.69 KB

Fixed the incorrect link to the administration theme page and the message formatting.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
FileSize
admin_theme_default.patch3.05 KB

Cool. I like that change a lot. Three small changes in this patch:

1. Rolled the patch from Drupal root, rather than in the system directory, per standards.
2. Instead of hard-coding the <em> tags around the administrative theme name, I instead used the %admin_theme placeholder, which places the output in a theme('placeholder').
3. I also added a sentence to further explain that the theme actually did apply, so the message now reads:

Please take note that the <a href="!admin_theme_page">administration theme</a> is still set to the %admin_theme theme. All non-administrative sections of the site, however, will show the selected %selected_theme theme.
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Needs review » Needs work

Er. one sec. that didn't work. ;)

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Needs work » Needs review
FileSize
admin_theme_default_0.patch3.46 KB

Ok, let's try that again. :)

Same patch, but changed the last sentece to...

...will show the selected %selected_theme theme by default.

Since users may override the default theme setting if they're given permission.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
FileSize
admin_theme_default_1.patch2.81 KB

Hm. No idea where that weird alert() stuff came in on checkboxes. Must've been some debugging from a previous patch.

webernet’s picture

webernet CreditAttribution: webernet commented

Perhaps a minor change to the wording:

Please note that the <a href="!admin_theme_page">administration theme</a> is still set to the %admin_theme theme, consequently, the theme on this page remains unchanged. All non-administrative sections of the site, however, will show the selected %selected_theme theme by default.

The changes in this patch should help make changing themes less confusing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
FileSize
admin_theme_default_2.patch2.49 KB

Works for me. That message is getting a little verbose now, but on the other hand it is terribly confusing without it. ;P

Also, I added a semi-colon before consequently, rather than a comma. I like semi-colons. ;)

edmund.kwok’s picture

edmund.kwok CreditAttribution: edmund.kwok commented
FileSize
admin_theme_default_3.patch2.45 KB

Patch rerolled against HEAD.

Yup, the message makes things clearer now :-). Hmm, will this be commited?

davemicc’s picture

davemicc CreditAttribution: davemicc commented
FileSize
admin_theme_default.patch.txt2.49 KB

Patch rerolled against HEAD.

I was also confused at first when I changed the theme and the admin pages didn't change. This patch handles things as I would expect.

Steven’s picture

Steven CreditAttribution: Steven commented
Status: Needs review » Fixed

Good patch, this will reduce a lot of confusion. Committed to 5.

yched’s picture

yched CreditAttribution: yched commented
Status: Fixed » Needs review
FileSize
themes.patch1.18 KB

Small correction :
typo ($values instead of $form_values)

And small improvement :
if the "new" default theme is the same as the admin theme, no need to display the message.

davemicc’s picture

davemicc CreditAttribution: davemicc commented

Nice catch. I missed that while testing because the sentence still makes sense without the selected theme name in there. This patch works for me.

webernet’s picture

webernet CreditAttribution: webernet commented
FileSize
patch_42.txt1.2 KB

Rerolled.

Looks good to me - Fixes a typo and eliminates a potentially confusing message if the "Administration theme" is the same as the new "Default theme".

yched’s picture

yched CreditAttribution: yched commented
Status: Needs review » Reviewed & tested by the community

RTBC, then...

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD.

Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)