fix: custom theme detection #10410
fix: custom theme detection #10410
Conversation
vladitasev
requested review from
dominikschreiber,
pskelin,
ilhan007,
nnaydenow and
maddel
| @@ -68,7 +68,7 @@ const processThemeMetadata = (metadata: ThemeMetadata): ThemeDescriptor | undefi | |||
| let baseThemeName; | |||
|
|
|||
| try { | |||
| themeName = metadata.Path.match(/\.([^.]+)\.css_variables$/)![1]; | |||
| themeName = metadata.Path.split(".")![2]; | |||
There was a problem hiding this comment.
the ! will eventually happen to be undefined, and we are back in the scenario where an exception is thrown and no theme is loaded. Can we provide a better fallback?
Also, this was the case before, but what if a custom theme has a . in the name? is it escaped? should we also check this?
There was a problem hiding this comment.
Custom themes can't have a . in the theme name, otherwise they couldn't even be saved by the theming engine.
Starting with SAP/theming-base-content@11.23.0, there is --sapSapThemeId, which might be a good fallback (is always an SAP theme, but that might be what you're after anyways):
themeName = getComputedStyle(document.body).getPropertyValue('--sapSapThemeId');
// is "sap_horizon" for SAP Morning Horizon,
// is "sap_horizon" for custom themes based on SAP Morning HorizonMaybe you don't even need JavaScript for that anymore, because you can target --sapSapThemeId with CSS container style queries – see Theme-specific CSS for your application.
There was a problem hiding this comment.
Thanks for the insight!
I changed the check a bit - if there are 4 parts (after splitting) then this means we're with the "known" format and themeId is guaranteed to be the third part.
However, if this is not the case (future versions of the metadata), we read the CSS var as this is the standard as of last release.
|
🎉 This PR is included in version v2.6.0-rc.3 🎉 The release is available on v2.6.0-rc.3 Your semantic-release bot 📦🚀 |
We cannot rely on the
Pathfield ending oncss_variables, we have to follow the<framework>.<lib>.<theme>.<field>pattern, therefore the custom theme name is the third part of the value, split by.