-
-
Notifications
You must be signed in to change notification settings - Fork 446
Admin theme changed by default config value #1216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Thank you for writing this bug report! There is something wrong in the theme switcher code. I will fix this as soon as possible. As long as this is fixed and included in the next release, you could just move the skin overwrite to the website level. |
@aterjung Thank you for the quick response. I think this issue is not new but we never noticed it because the (old) default theme it is falling back to was the expected behavior. If I remember correctly from debugging it, the code responsable for this behavior was not edited. You even may say that it's a feature, but if it is than it is a confusing one ;-). Thank you for noticing that workaround, we already did do exactly that for now. So no big problem for now but it makes the configuration a bit more confusing. |
@AlterWeb where is the code you identified as source for this error? I am a little bit confused at the moment, my dev-environment where the theme switcher was developed does not show this behavior! But all of my customers live systems do have this error. |
I don't have access to my workstation at the moment (typing this on my phone) but if I remember correctly it was in the class Mage_Adminhtml_Controller_Action. Around line 163 there is a foreach loop setting those values. I can check this tomorrow. |
@aterjung I just checked it and this is indeed where those values are set. So this code:
|
I think it's worth considering making the new openmage theme as a separate Make a new openmage package
Change
|
@kiatng thank you for this. This would be a solution for #1164 at least. I will prepare a fork for easy testing. I am not shure if this can fix the strange behavior of the Mage_Adminhtml_Controller_Action`s preDispatch method. Probably i miss the point that is handled here:
For me it looks like it handles the selected theme layout/template/skin/locale settings for the backend. getConfig()->getNode should get the values form xml config. Indeed the returned values are the ones for the frontend (defined in core_config_data) if there are no xml definitions. I wasn't aware of this fallback until now. This happens as well for the package name. If you remove
the package you define in the backend will be used for the adminhtml area! Setting default values in the config.xml fixes this but breaks the theme switch that is implemented now:
Thus, i think migration to a backend theme package let this problem disappear without resolving it. If i get it right, this is a problem of bad design back to the stating days of magento. :-) The backend theme uses the config of storeId 0 which is the "default config" everybody uses to put the fronted theme in. To set it fixed to "adminhtml" this value is overridden by xml config - but only for the package - and "default". If you add a "skin"-Overwrite (for the frontend) in the backend it is not found in adminhtml and therefore ignored falling back to "default". Do we really need the layout, skin, template and locale overwrite for backend themes? Probably we should drop this foreach an fix the values there? |
Continue with my comment, I tested custom theme rewrite based on it. Custom Theme Rewrite in Custom PackageAs an example, I want to rewrite this file Copy and paste the file to new package called Then I added this: Add theme.xmlAdd a new file <?xml version="1.0"?>
<!--
/**
*
* @category design
* @package ccs_default
*/
-->
<theme>
<parent>openmage/default</parent>
</theme> Note: if the file is missing, it will fallback to the package Set the Custom PackageIn any <stores>
<admin>
<design>
<package>
<name>ccs</name>
</package>
<theme>
<default>default</default>
</theme>
</design>
</admin>
</stores> Voila: Issues
|
@kiatng have you read my comment about the way the adminhtml package is forced in the Controller? Mage::getConfig()->getNode('stores/admin/design/package/name') will return this line from core_config_data if no package is defined for store/admin in any config.xml: That is because the admin area is considered as store "0". The Problem is that the "default" config for the frontend themes is also saved with scope_id 0. Normaly there will always be a config.xml setting the adminhtml package to a different value. If you put a overwrite for "template" for example in the backend config in "default" scope, lets say "mytemp", then this value form db will be set for the backend theme as well - but it is intended for frontend. That is becase no config.xml setting is made for the template path. If you use the default theme, that will not harm, cause there is no folder "mytemp" in the adminhtml folder which let the value gracefully fall back to "default". But using the openmage theme, the fallback to "default" is not what we want. The change to an own package you suggest will heal this symptom of a broken design at first, because the theme would be called "default" again. But everybody wo builds his own custom theme by adding a theme to the openmage package would ran into this problem again. From my point of view, we have to fix this "combined" config path for frontend and backend theme to get this fixed. If the openmage theme should be a package or a theme is a different discussion then. |
@kiatng I tested this with the same results. I build up theme packages just like you showed and created a custom package which I set as design package from config.xml file!
Going on from this point, my PR #1229 reverts the changes to Mage_Adminhtml_Controller_Action::preDispatch method for the legacy theme switcher and introduces a new option to overwrite admin theme from backend. This is probably not the final version, but it should work without problems for your example and let you chose which design package you want right from the backend. It would be great if you could confirm that this works for you.
|
We use the below approach for several years without problems for our own packages and themes. Now we have the problem with the new Openmage theme too and solved it like this:
Add theme.xml in openmageWe use in file: <?xml version="1.0"?>
<!--
/**
*
* @category design
* @package openmage_default
*/
-->
<theme>
<parent>default/default</parent>
</theme> Setting the package and theme in config.xmlIn <stores>
<admin>
<design>
<package>
<name>default</name>
<openmage>openmage</openmage>
</package>
<theme>
<default>default</default>
</theme>
</design>
</admin>
</stores> In // get legacy theme choice form backend config
if (Mage::getStoreConfigFlag('admin/design/use_legacy_theme')) {
$package = Mage::getConfig()->getNode("stores/admin/design/package/name");
} else {
$package = Mage::getConfig()->getNode("stores/admin/design/package/openmage");
}
Mage::getDesign()
->setArea($this->_currentArea)
->setPackageName((string)$package)
->setTheme((string)Mage::getConfig()->getNode('stores/admin/design/theme/default'));
foreach (array('layout', 'template', 'skin', 'locale') as $type) {
if ($value = (string)Mage::getConfig()->getNode("stores/admin/design/theme/{$type}")) {
Mage::getDesign()->setTheme($type, $value);
}
} This is working for us. Known issue:There is some problem with the fallback logic because the skin from Update 2023-04-23After some investigation and reading the very helpful information from Alana Storm and Eric Wiese to "Infinite theme fallback", I've found a working solution without breaking backwards compatibility. First of all: All the stuff from above is needed.
<?xml version="1.0"?>
<!--
/**
*
* @category design
* @package openmage_default
*/
-->
<theme>
<parent>default/default</parent>
<layout>
<updates>
<openmage_adminhtml_default>
<file>openmage.xml</file>
</openmage_adminhtml_default>
</updates>
</layout>
</theme>
public function getFallbackScheme($area, $package, $theme)
{
$cacheKey = $area . '/' . $package . '/' . $theme;
if (!isset($this->_cachedSchemes[$cacheKey])) {
//First we have to check if theme exists
$path = "$area".DS."$package".DS."$theme";
$fallback = false;
if (!is_dir(Mage::getBaseDir('design') . DS . $path)) {
//Fallback to default
$theme = (string)Mage::getConfig()->getNode('stores/admin/design/theme/default');
$fallback = true;
}
if ($this->_isInheritanceDefined($area, $package, $theme)) {
$scheme = $this->_getFallbackScheme($area, $package, $theme);
} else {
$scheme = $this->_getLegacyFallbackScheme();
}
if ($fallback) {
$first = array_shift($scheme);
$scheme = array_merge([$first], [['_package' => $package, '_theme' => $theme]], $scheme);
}
$this->_cachedSchemes[$cacheKey] = $scheme;
}
public function getFileLayoutUpdatesXml($area, $package, $theme, $storeId = null)
{
if (null === $storeId) {
$storeId = Mage::app()->getStore()->getId();
}
/* @var Mage_Core_Model_Design_Package $design */
$design = Mage::getSingleton('core/design_package');
$layoutXml = null;
$elementClass = $this->getElementClass();
$updatesRoot = Mage::app()->getConfig()->getNode($area.'/layout/updates');
//Newly added this function from Eric Wiese fix
$updatesRoot = $this->addFallbackThemesLayoutUpdates($updatesRoot);
Mage::dispatchEvent('core_layout_update_updates_get_after', array('updates' => $updatesRoot));
$updates = $updatesRoot->asArray();
...
/**
* Add layout files added via theme.xml to layout updates
* for all themes that are parents of this theme.
* Observes: core_layout_update_updates_get_after
*
* @param Varien_Event_Observer $observer
*/
public function addFallbackThemesLayoutUpdates(Mage_Core_Model_Config_Element $updates) {
/* @var $designPackage Mage_Core_Model_Design_Package */
$designPackage = Mage::getSingleton('core/design_package');
/* @var $fallback Mage_Core_Model_Design_Fallback */
$fallback = Mage::getModel('core/design_fallback');
$fallbacks = $fallback->getFallbackScheme($designPackage->getArea(), $designPackage->getPackageName(), $designPackage->getTheme('layout'));
for($i=count($fallbacks)-1; $i>=0; $i--) {
$fallback = $fallbacks[$i];
if(!isset($fallback['_package']) || !isset($fallback['_theme'])) {
continue;
}
$fallbackPackage = $fallback['_package'];
$fallbackTheme = $fallback['_theme'];
$themeUpdateGroups = Mage::getSingleton('core/design_config')->getNode("{$designPackage->getArea()}/$fallbackPackage/$fallbackTheme/layout/updates");
if(!$themeUpdateGroups) {
continue;
}
foreach($themeUpdateGroups as $themeUpdateGroup) {
$themeUpdateGroupArray = $themeUpdateGroup->asArray();
foreach($themeUpdateGroupArray as $key => $themeUpdate) {
$updateNode = $updates->addChild($key);
$updateNode->addChild('file', $themeUpdate['file']);
}
}
}
return $updates;
} Now it is possible to set custom values in |
@theroch Great job. Just some thoughts. Wouldn't it be easiest to add a configuration section like the one for the frontend instead of using a static configuration by dropdown to enable the new theme? At the end we could offer alternative design packages for the backend and it would be also easier for less experienced users to install new themes. |
Sure, this is possible, but I think this is out of scope of this topic. If you want such a feature then please create a feature request.
Yes, this possible too, all the necessary logic is already implemented. But from my experience, it is better to use a standard adminhtml theme for clients/merchants. It is easier to create training courses and users can use the internet to find useful information about the handling and functions of the backend. |
Can I ask what is an official way to override admin default templates? Should I use |
@theroch had you time to work on this - or share a branch to review? |
Preconditions (*)
Steps to reproduce (*)
Expected result (*)
Actual result (*)
The text was updated successfully, but these errors were encountered: