In some modules the way we set things in the theme is hardcoded. We should always use thinkgs like activetheme... also checking on themenames such as 'socialsaas' in code should never be done. Better check for the base theme of the current theme being socialbase/blue.

Check code for checks such as these.

CommentFileSizeAuthor
#2 active_theme-2923371-2.patch3.13 KBslowflyer

Comments

jochemvn created an issue. See original summary.

slowflyer’s picture

slowflyer commented
StatusFileSize
new active_theme-2923371-2.patch3.13 KB

Hi Jochem,

after we already solved parts of that issue on:

https://www.drupal.org/project/social/issues/2907743

I looked on the work left over.

I looked at all occurences of "socialsaas" and "->get('system.theme')" in the code.

Only:
social_activity.install
socialblue/theme-settings.php
social_core.install

still had the old hard coded syntax to check for socialsaas or socialblue.

I replaced it in:
social_activity.install
socialblue/theme-settings.php
and improved / corrected the comment in improved_theme_settings.module

This is reflected in the patch attached.

I did not change social_core.install, because during install, there is no need to check for theme inheritance.

The code only checks for socialbase as basetheme. There is no need to check for socialblue as well, because socialblue will have still have socialbase as basetheme and

\Drupal::service('theme.manager')->getActiveTheme()->getBaseThemes()

returns all basethemes used to build the current theme.

jochemvn’s picture

jochemvn commented
Related issues: +#2924676: Warnings from improved_theme_settings_page_attachments after update from 1.5->1.7

Hi Ulrich,
That 's really great. I would also like to fix the issue from: https://www.drupal.org/project/social/issues/2924676 in this story. There should be some kind of hierarchy, that if for your current theme there are not settings, the base theme should either take over. That way we don't have to worry about subthemes not having certain values defined in the basetheme(s)
Jochem

tc33133’s picture

tc33133 commented

Jochem,
I think this is a great idea, and it would be helpful to include block layout as well. For instance, using a custom subtheme, I'm unaware of the Social Private Message stuff, unless I switch to socialblue theme. Even if its some text included in the Changelog or some instruction on how the new components are integrated.
Thanks!

jaapjan’s picture

jaapjan commented
Status: Active » Fixed

I think this is already done? As far as I know there is no reference anymore and they have been replaced in the issue slowflyer linked.

tc33133, you are right. That would be a great addition, but perhaps should be handled in a different story?

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.