| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | theme system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
Conversely, what about a theme that is only intended to be an admin theme?
While having themes that are available as both is a good thing, I think it's time that we allow themes to restrict themselves if they want, to not be set as an administration theme. Contributing and supporting a theme that supports hundreds of modules is prohibitive enough, but supporting their backends as well is even more of a barrier, especially since some contrib modules can have, um, 'special' UIs.
Especially with Seven in core, the pressure for themes to be fully admin-supportive seems to be somewhat alleviated.
What about an optional line in the theme's .info file which could specify this? With "both" being the default if not included.
Related issue, since the UI is changing in #491214: implement the top level Appearance / Choose Theme admin page, and would need to have the links reflect what can be enabled for what.
Comments
#1
So I spoke to JohnAlbin about this as well. It is indeed somewhat wierd to require modules, which live solely in the administration space to work for interfaces such as Garland, and whatever new core theme.
However and this is sturdy said, the use case for both is going to be awkward. I'd rather have something that favors, administration theme - to be able to say, this is only an administration theme. Then themes to say, I am both or not.
#2
Hey Bojhan, thanks for your comment. I'm afraid I'm not 100% clear on what your conclusion is stating. Are you saying you support it as a general concept but not that themes should be allowed to restrict this?
#3
I support :) I think, this is a good change - however we should definitely look at all the technicalities of this.
#4
Well, as far as #491214: implement the top level Appearance / Choose Theme admin page evolves, it might not have links directly in the main theme setup screen. Also, it could easily be a WTF for users if they cannot turn certain themes to be an admin theme or front end theme without further explanation.
#5
can't help but get the feeling this is a step in the wrong direction, because this would take away the pressure to use generic css techniques and it makes it very tempting to limit support significantly
also it would require quite some work, since an admin theme basically overrides the default theme whenever the context is considered administrative, so what if there's no admin theme active and the current default theme is one that's "not to be used as admin theme", should it then fall back on the bare defaults (these: http://api.drupal.org/api/file/modules/system/page.tpl.php/7/source http://api.drupal.org/api/file/modules/node/node.tpl.php/7/source http://api.drupal.org/api/file/modules/block/block.tpl.php/7/source) or should it attempt to use the default theme anyway?
imo, the opposite makes a whole lot more sense, half the true admin themes out there weren't made to function as a default theme anyway
#6
"because this would take away the pressure to use generic css techniques and it makes it very tempting to limit support significantly"
That's what goes in the "plus" category for me ;)
Unless by "generic" you meant "standard", but I don't think that is the case. It *does* mean that designers and themers can get away from the Drupally look more when creating themes. There are themes/designs out there that couldn't really be admin themes in practice anyway, but if you give them the ability (and the default is to use it everywhere), the end users *will* try, even if you tell them not to. This is simply making that social/documentation convention a technical restriction so users don't think their site/theme is broken.
I don't think "you must support the administrative interfaces of X thousand contrib modules" was ever in the intended spirit of creating a contrib theme.
#7
@stephthegeek: "you must support the administrative interfaces of X thousand contrib modules" I think was part of the original goal, but the administrative interfaces grown so complex that it is not too feasible, I'd agree.
#8
Just to provide some specific context, a perfect example is Acquia Slate (or any dark theme). Since it has a dark background with light foreground text/links, any module which specifically sets a light background anywhere (say.. Views/Panels. or anything with a light table, coloured divs, tabs, different gallery modules, etc) and doesn't also set a dark text/link colour on these will render its text nearly invisible, which then understandably get reported as theme bugs.
Now, the proper thing to do would be for those modules to make sure they also specify a foreground colour, but they rarely do since the vast majority of themes are light. In order to "fix" this in the theme, we'd have to jump in on each of these modules' class/ID declarations that set backgrounds and also set foregrounds.
That to me is a perfect example of why it's not really feasible.
I'm all about leaving it as an option, and even the default for themes to be available as administrative themes too. But I think it inhibits creativity and means that themes can be incorrectly thought to be broken if they don't support the admin section.
#9
I see your point, and I'm not going to pretend I know more about what making a generic theme that works for everything is about, I usually make a theme based on a design and a spec, so they pretty much only fully support the modules running on that site, and pushing this change wouldn't affect me at all, personally
in your particular example, I'd say it's the module's default theming that doesn't make a lot of sense and I'm sure that there's cases where it's lot more murky
but then why do we have admin themes at all?
#10
The problem with admin pages is that you can't rely on generic stylings. In order to make admin pages look consistent with other parts of the website you have to write a lot of very specific styles. This is especially true with Drupal 7 and modules such as overlay or dashboard. Needless to say that the markup and core stylesheets on admin pages are awful.
Assuming that all themers will be adding support for admin pages is at least overenthusiastic.
#11
Here's a very simple patch that would allow a theme to declare itself an admin-theme by adding
type = admin-themeto it's .info file.
All the patch does is then not display the link "Set default" for the theme on /admin/appearance. Inexperienced users are then not able to enable an explicit admin theme as their default theme.
In effect this patch adresses seutjes comment from #5
The patch writes this line to Seven theme's .info file to illustrate the effect - and yes, the condition in system.admin.inc could probably be written much better.
#12
bot.
#13
Cant we have it both ways - so we can declare either or both?
#14
This patch also checks if
type = admin-themeis set for determining the content of the select box where users can select their admin-theme.- The default theme is always there to be selected
- Themes which don't declare
type = admin-themein their .info file are not added to the select list.This would mean that theme authors would explicitly have to declare in their .info file whether they wish their theme to be usable as an admin theme out of the box. Developers would still be able to add this declaration to whatever theme they want.
Also, this would of course have to be documented and probably a test written for.
#15
The last submitted patch, settings-admin-theme-1.patch, failed testing.
#16
Hm... this was stupid.
This patch is better:
We now have two possible values for themes
- type = admin-theme puts a theme into the admin theme select box and disables setting it as default.
- type = frontend-theme removes a theme from the admin theme select box and enables setting it as default.
- Themes with no declaration can be a default or an admin theme
So now whenever a theme developer wants to either define his theme as exclusively an admin theme or a frontend-theme, he can do so.
EDIT: The changes in garland.info and seven.info serve merely to illustrate the functionality and have to go out of course. Also, they might trigger some interesting testbot-failures :-)
#17
A) I think this is a hugely awesome idea and should definitely be part of core
2) Sadly, I have a feeling this will be relegated to D8 - I cant think of any way to spin this as anything other than a (super-cool) new feature
III) Patch review:
Shouldn't this:
+ if (empty($theme->info['type']) || !($theme->info['type'] === 'frontend-theme')) {be
+ if (empty($theme->info['type']) || ($theme->info['type'] !== 'frontend-theme')) {(and the same later on with
($theme->info['type'] !== 'admin-theme'))#18
@bleen: I share your premonitions :-(
However, it's not an API change, easily understood, easily documented and might just slip in. It doesn't change anything for existing themes but might prevent lots of WTFs, when new users enable Seven as the default theme and are disappointed. Also: More power to the themers!
New patch attached for completeness' sake. This time without the changes in garland.info and seven.info so it could be committed as is. Also renaming.
Plus a screenshot of the *new* /admin/appearance when garland is defined as a front-end theme (can't be selected as admin theme) and Seven is defined as an admin theme (can't be set as default).
Crossing fingers and whistling in the dark...
#19
Whoa, this does not look right. There are no indicators why Seven can not be set as default. To me its pretty clear that we need to think about designing that page to hold "normal" themes and "admin" themes. I am fine with not even displaying Seven, and just having it in the drop down.
#20
This patch is clearer by taking out all the negations with
!==and thusly making the code easier to understand.Documentation would have to go here, I guess: http://drupal.org/node/171205
EDIT: Sorry Bojhan, didn't read your last comment before posting this.
#21
If we scale this back to just preventing a theme from being set as an admin theme perhaps we don't need a UI change, which is certainly going to get this bumped to D8.
Therefor:
- Any theme can be enabled and set to default.
- Not all themes will appear in the Administration themes list.
To do this we would need to:
1 - Add the "type = frontend-theme" option for themes that do not want to be admin theme.
2 - Remove the "Default theme" option.
3 - Add some documentation.
4 - ?
I am changing this to a bug because at the moment any theme can be set to the admin theme, which can cause the site to become unusable, or at the very least important admin pages can become unusable. We should remember that in D7 themes have access to things like hook_page_alter, or even the new proposed hook_system_region_list_alter. Allowing any theme to be set as an admin theme is a major WTF and UX fail.
#22
Moving to 8.x (which could be backported to 7.x).
I prefer adding the “type=admin” line in the info files only for admin themes.
We don't need to add the type line to every single theme since that by their majority they are all front-end theme anyway. We just need to highlight the few exceptions that are indeed of type admin. That way we could hide the few admin themes behind a tab.
#23
re #22: I disagree ... this issue is as much about preventing admin themes from being chosen as the front-end theme as it is about front-end themes being chosen as the admin theme. The former is just plain common sense. A theme like Rubik is simply meant for admin tasks only, so why offer that to users when choosing their front-end theme. The later (as Jeff pointed out in #21) is skating very close to the line of "bug" because a user who chooses most of the contrib themes as their admin theme risk loosing some admin abilities.
That all said, the patch in #20 is careful not to require that a theme add "type" to its info file so we will not be forced to change all of those themes out there; in that case the theme will simply be treated as "both."
Attached is a reroll of #20 with a couple minor changes:
- made "seven" an admin theme
- added an "administrative only"|"front-end only" note to the theme name.
As a side note ... man that theme admin page needs some work!
#24
The last submitted patch, adminonlytheme-550102.patch, failed testing.
#25
This patch fixes the tests...
#26
I do agree. But why special-case all themes (frontend vs admin)?
+++ b/modules/system/system.admin.inc@@ -222,6 +228,15 @@ function system_themes_page() {
+ if (isset($theme->info['type'])) {
+ if ($theme->info['type'] == 'admin-theme') {
+ $theme->notes[] = t('administration only');
+ }
+ if ($theme->info['type'] == 'frontend-theme') {
+ $theme->notes[] = t('front-end only');
+ }
+ }
+
We need to identify and mark only the admin themes so that:
In other words, all “admin” themes should be invisible on the main Appearance page, or visible only on the Administation theme fieldset (or behind a localtask tab).
Powered by Dreditor.
#27
I see. We have 3 types of themes.
We should special-case only the later two.
type=admin-compliant
type=admin
For the remaining (front-end) themes and new theme, we should not require the “front-end” type.
Admin-compliant themes would show on the Appearance page and be available in the Administrative theme section.
#28
#27 The trouble with this approach is that any possible-admin theme needs to be explicitly marked as that. Or, if I made an frontend contrib theme, I can say: "I do not support this theme being used in admin, but if you want to use you are allowed to", but I cannot do that with your theme-types, because if I a flag my theme as an admin-compliant, then users will expect me to support it, and if I don't, users will have no way to selectt it as admin at their own risk.
In other words, the theme type attribute might set exclusive-use cases only, like this theme is indeed to be used as admin-only or frontend-only, but if the themer don't set this attribute, the theme will work as a regular theme works today.
#29
I agree with #28 ... setting back to needs review
#30
#1167444: Not clear which themes are enabled and disabled proposes a redesign of the Appearance page, it will need to take into account changes made by this issue. I tend to like the idea of additional labels but I'm not sure that its enough or that they're being implemented in an optimal way - we need more visual separation between admin and front end themes. We should work on the UI in the other issue.
As for this patch - personally I'm not a big fan of the hyphenation, and would prefer something like:
type = admin themetype = frontend theme
#31
type = admin themetype = frontend theme
... why do we even need to say 'theme'? We're in a theme .info file; the context is clear :)