Users with "select different theme" permission have a theme selection table at their account edit page. That table is not displaying the theme name and description like the listing at the admin page, but theme filename and path.

Themes are ordered by filename and not by display name.

The patch fixes this and display a theme selection table quite similar to the one displayed at the themes admin page.

Related issue: http://drupal.org/node/201641

Note for developers:
- I have moved the function system_sort_modules_by_info_name from the file system.admin.inc to the file system.module beacuse the function was not accessible from system.module at hook_user

Notes for testers:
- Be sure to be logged with a user that has "select different theme" permission.
- Enable more than one theme.
- Go to the user account edit page.
- Review the theme selection table under then Theme configuration fieldset

This patch uses the same approach used here to sort the themes by name.

Please test.

CommentFileSizeAuthor
#11 user-theme-configuration.png23.31 KBdropcube
#11 user-account-themes-listing-211051-11.patch6.97 KBdropcube
#9 Picture 1.png55.27 KBdvessel
#7 user-account-themes-listing_7.patch6.16 KBpancho
#1 user-account-themes-listing_1.patch5.39 KBpancho
user-account-themes-listing.patch3.91 KBdropcube

Comments

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented
StatusFileSize
new user-account-themes-listing_1.patch5.39 KB

Here is a revised version:

  • system_theme_select_form: removed copying $themes array to $enabled, as this is not needed.
  • system_theme_select_form: moved uasort to where ksort was before - if there is only one theme there is no need to sort.
  • system.module: fixed "\ No newline at end of file" problem.
  • theme_system_theme_select_form: moved code back into the if-clause - there is no need for an if... continue construct.
  • some minor codestyle fixes.

Moving system_sort_modules_by_info_name to system.module is a good thing.
Patch tested, works as expected.

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented
Assigned: dropcube » pancho
Status: Needs review » Needs work

THREE more things to take care about:

  • This little remark down there in the theme information doesn't make it obvious enough which theme is the site default. We might want to add '('. t('site default theme') . ')' right after the theme name instead.
  • I'm not sure whether it is always wanted that regular users know so much about the inner workings of the theme. Also, the description is neither translateable (and we can't add a new string during string freeze) nor admin changeable, so we just can't expose it to users!
  • The code is still too complicated IMHO.

I'll follow up with a patch!

dropcube’s picture

dropcube commented
Assigned: pancho » dropcube
Status: Needs work » Reviewed & tested by the community

Ok, tested the revised version and works as expected as well.

RTBC as far as I can see.

dropcube’s picture

dropcube commented
Assigned: dropcube » Unassigned

I assigned this to me accidentally. Pancho assign the issue to you again if you prefer ;)

dropcube’s picture

dropcube commented
Status: Reviewed & tested by the community » Needs work
dropcube’s picture

dropcube commented

I'm not sure whether it is always wanted that regular users know so much about the inner workings of the theme. Also, the description is neither translateable (and we can't add a new string during string freeze) nor admin changeable, so we just can't expose it to users!

I agree with this and makes sense. For regular users is enough with the screen shot and the theme name, IMHO.

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented
Status: Needs work » Needs review
StatusFileSize
new user-account-themes-listing_7.patch6.16 KB

Didn't manage to improve the code without larger changes. Postponed that.
The rest I did, and also I added this 'uasort'ing to system_menu(), where it is also needed.
Now this should be fixed.

dropcube’s picture

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

Ok, tested this one.

- The theme listing at the account edit page looks OK now.
- The tabs at admin/build/themes/settings are ordered correctly as well.

RTBC as far as I can see.

dvessel’s picture

dvessel commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new Picture 1.png55.27 KB

Doesn't look like it. Default radio button not selected if the site's default is your accounts default. And no description.

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented

The "no description" thing is intended (see #2). The default radio thing I'll have to take a look at tomorrow!

dropcube’s picture

dropcube commented
Assigned: Unassigned » dropcube
Status: Needs work » Needs review
StatusFileSize
new user-account-themes-listing-211051-11.patch6.97 KB
new user-theme-configuration.png23.31 KB

For me, when an user select a theme it means that s/he want to use this theme, doesn't matter what theme the administrator has set as the site default theme. If a user has the rights to select a different theme, the theme should be used for him/her even when the admin has changed the site default theme.

To better understanding what I am trying to say, let's reproduce the following situation:

- The user goes to their account edit page and select the theme s/he prefers. For example, select Garland, which is in fact the site default theme in that moment. But that doesn't mean that s/he want to use the site default theme, s/he may prefer to use Garland always, even if it is not the site default theme.
- The user submits the form and no theme selection is saved because s/he selected the site default theme (but s/he doesn't want to use the site default theme, s/he just want to use Garland always)
- The admin changes the site default theme to other theme, let's say Chameleon.
- When the original user accesses the site, Chameleon will be used because the user account doesn't have an user theme defined (an empty string was used for the site default theme key in that moment), and yes, s/he just wanted to use Garland always.

This is not a situation that happens every day, but may happen and may frustrate users, and in fact, is a bug.

IMHO, users should be able to select a theme to use throughout the site and should be able to select an option like: "Default site theme". Selecting this option, the user theme will be always the site default theme, and will avoid further confusions . See attached image

The patch implements this and the problem reported in #9 get fixed.

Please comment/suggest.

dvessel’s picture

dvessel commented

I see what your saying. It seems account specific themes are seldom used so we never talked about statefulness –is that even the right word? of how it holds. I'm curious how Drupal 5 behaves in this regard.

Judging by the screen shot it doesn't fit IMO. Radio buttons can only have one default. When you select the site default, which gets selected when you revisit the page? There would be two choices that can be enabled but radios don't allow that and it's better to use the radio as an indicator of what theme is being used. A "site default" checkbox makes more sense. If enabled, the default radio button follows the sites default. Disabled, then it doesn't.

dropcube’s picture

dropcube commented

When you select the site default, which gets selected when you revisit the page?

"Site default theme" is intended to mean: "No, thank you. I do not want to use any of those themes, I just want to use the theme that the admin selects as the site default theme. If the admin changes the site default, I want to use the new one selected... " ;)

So, the site default them is showed and the "Site default theme option" get checked. Makes sense ?

If the user select other theme, then that means that s/he wants to use the selected theme, even when the admin changes the site default theme.

Currently, if the user who has select the site default theme as his account default theme goes to the account edition page and submit the form (to change any other info) the user theme selection is overwritten (beacuse an empty string is used for the site default theme).

The patch at #11 fixes this.

dvessel’s picture

dvessel commented

Any theme enabled by the admin will show in the list. So, if the default theme is Garland and you select "Site default theme". There you have Garland in the list unselected. Use the radio button as an indicator of what is actually being used. The checkbox serves as a modifier to follow the sites' default or to use your account specific theme. But always have the radio select default to the current theme.

Look at all the other forms with radios. They are used when only one value can be returned but they are all unique. Your mudding up what the radio selection means by adding another that's not a unique theme. Just a different condition of an existing value.

dropcube’s picture

dropcube commented

They are used when only one value can be returned but they are all unique. Your mudding up what the radio selection means by adding another that's not a unique theme. Just a different condition of an existing value.

Radio buttons are used to select only one option from a list of exclusive options. The users may select a theme or just decide to use the site default whatever it is. So, the values are exclusive. The radio buttons indicate the user choice, not the current default theme.

Now, from a coding perspective, each of the options have different values. The value of the last (where the user may choice to use the site default theme) is an empty string, which means that the user does not have an account specific theme.

I am not totally sure about the interface, I actually think that it may be confusing for users. Adding some help text may help.

Please, comment/suggest.

dvessel’s picture

dvessel commented

dropcube, thanks for the effort. But we might be running into 7 territory here. How about leaving it so it behaves like 6 for now and we can really fine tune it in 7.

Any minor tweaks I wouldn't mind but we are really late in the cycle.

dropcube’s picture

dropcube commented

Well, leaving this as it is now we have two problems:

- If the user theme selection is the actual site default theme and an empty key is used for the site default theme, then the user theme selection will be overwritten when the user submit the form.
- The user does not have the possibility to say in a moment: "Well, I don't want to use any of these themes, I just want to use the site default theme whatever it is" or "I want to use Garland, always, if Garland is set by the admin as the default theme, good, but if the admin changes the theme, I want to continue using Garland".

dvessel: What do you suggest ? What do you think is the better choice for Drupal 6 so that I can update the patch ?

dvessel’s picture

dvessel commented
Status: Needs review » Needs work

The two points are directly related, right? Well, on the second point. Although minor, I think it's definitely a usability problem. Not really a bug IMO so I think we should hold off on that for D7.

This would be a minor irritation at best when the site's default changes which I think is seldom in most sites. Also, this is how D5 behaves now. Never seen an issue about this before.

This issue started about mimicking the theme admin pages, no?

jonathan1055’s picture

jonathan1055 commented

I guess nothing got committed in D6 yet, as at D6.8 the file directory structure is still being displayed to the users. I would say this is worse than showing the theme description.

Although I admit that the theme desc can't be changed by admins or translated, because it is fixed in the theme.info file. Maybe we need another field which the admin can enter as the descriptive text for the users to see? It is very useful to be able to give some extra text in addition to the theme name, to give the user background as to why they may want to chose one theme over another.

dropcube’s picture

dropcube commented
Assigned: dropcube » Unassigned

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.