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.

Comments

webchick’s picture

Assigned: Unassigned » webchick
Status: Active » Needs review
StatusFileSize
new1.75 KB

Here's a patch.

dries’s picture

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

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

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

StatusFileSize
new1.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

StatusFileSize
new1.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

StatusFileSize
new2.04 KB

Perhaps a message like this?

edmund.kwok’s picture

StatusFileSize
new2.69 KB

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

webchick’s picture

StatusFileSize
new3.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

Status: Needs review » Needs work

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

webchick’s picture

Status: Needs work » Needs review
StatusFileSize
new3.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

StatusFileSize
new2.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

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

StatusFileSize
new2.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

StatusFileSize
new2.45 KB

Patch rerolled against HEAD.

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

davemicc’s picture

StatusFileSize
new2.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

Status: Needs review » Fixed

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

yched’s picture

Status: Fixed » Needs review
StatusFileSize
new1.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

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

StatusFileSize
new1.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

Status: Needs review » Reviewed & tested by the community

RTBC, then...

drumm’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD.

Anonymous’s picture

Status: Fixed » Closed (fixed)