HI
After updating from OpenSocial 1.5 -> 1.7 I'm getting the following warnings popping up:

Warning: array_flip(): Can only flip STRING and INTEGER values! in Drupal\Core\Entity\EntityStorageBase->loadMultiple() (line 227 of core/lib/Drupal/Core/Entity/EntityStorageBase.php).
Drupal\Core\Entity\EntityStorageBase->loadMultiple(Array) (Line: 212)
Drupal\Core\Entity\EntityStorageBase->load(NULL) (Line: 512)
Drupal\Core\Entity\Entity::load(NULL) (Line: 48)
social_font_render() (Line: 278)
improved_theme_settings_page_attachments(Array) (Line: 297)
Drupal\Core\Render\MainContent\HtmlRenderer->invokePageAttachmentHooks(Array) (Line: 273)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object) (Line: 117)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object) (Line: 108)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object) (Line: 158)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 657)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Warning: A non-numeric value encountered in improved_theme_settings_page_attachments() (line 294 of profiles/contrib/open_social/modules/custom/improved_theme_settings/improved_theme_settings.module).
improved_theme_settings_page_attachments(Array) (Line: 297)
Drupal\Core\Render\MainContent\HtmlRenderer->invokePageAttachmentHooks(Array) (Line: 273)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object) (Line: 117)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object) (Line: 108)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object) (Line: 158)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 657)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Warning: array_diff(): Argument #1 is not an array in improved_theme_settings_page_attachments() (line 372 of profiles/contrib/open_social/modules/custom/improved_theme_settings/improved_theme_settings.module).
improved_theme_settings_page_attachments(Array) (Line: 297)
Drupal\Core\Render\MainContent\HtmlRenderer->invokePageAttachmentHooks(Array) (Line: 273)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object) (Line: 117)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object) (Line: 108)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object) (Line: 158)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 657)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

If I switch from my custom theme to socialblue, 2 of the warnings disappear, but the
Warning: A non-numeric value encountered in improved_theme_settings_page_attachments() (line 294 of profiles/contrib/open_social/modules/custom/improved_theme_settings/improved_theme_settings.module).

remains.

Obviously its something theme related, but having a hard time running it down. Any ideas?

CommentFileSizeAuthor
#11 theme_settings-2924676-11.patch2.23 KBmaikelkoopman
#4 theme_settings-2924676-4.patch1.43 KBslowflyer

Comments

tc33133 created an issue. See original summary.

fraganda’s picture

fraganda commented
Component: Code (back-end) » Front-End

I have the same issue on the standard Social Blue theme, after upgrading from 1.6 to 1.7

Warning: A non-numeric value encountered in improved_theme_settings_page_attachments() (regel 294 van /.../profiles/open_social/modules/custom/improved_theme_settings/improved_theme_settings.module)

tc33133’s picture

tc33133 commented

I think I solved this, and it was due to my subtheme not matching new improvements in socialblue.

First, I copied theme-settings.php to my subtheme (and also deleted similar code I put in the .theme file that handled the theme settings stuff).
Then, I added a value for card border radius on the theme settings page.

That got rid of 2 of the warnings.

I also copied the color/ directory and its contents from socialblue theme to my custom theme.

That got rid of the 3rd warning.

slowflyer’s picture

slowflyer commented
StatusFileSize
new theme_settings-2924676-4.patch1.43 KB

I can only reproduce the error Frank could reproduce in #2.

The patch applied should solve it.

theme_get_setting() returns NULL, if the setting is not available. Checking for || [variable] == 0 in the if condition will be true, because 0 == NULL and caused the error.

The other errors are a little harder ...

If socialblue gets some new features in future releases and features are missing in a subtheme build on an older version od socialblue, its really hard to get things right.

1. idea: take the values from the new socialblue theme -> but what if socialblue theme is disabled?
2. idea: implement default values to use

This is a question I'm struggling with over last month. Should I start build a "general" subtheme as I want to have it today. Or wait for a project and build subtheme on project needs?
No matter what I do, there is no clean process to add new features added to socialblue to my subtheme as well.

fraganda’s picture

fraganda commented

I did get rid of the warning message in #2 by entering a value for the Card border radius in the Social Blue theme.

slowflyer’s picture

slowflyer commented

@fraganda: That's obvious. The message was caused by trying to divide NULL by 2. With defining a Card border radius you had numeric value.

But why are the radius properties part of socialblue instead of socialbase? If all new "features" would be added to socialbase, it would be much easier to have a fallback to values from basetheme.

socialnicheguru’s picture

socialnicheguru commented
Status: Active » Needs review
maikelkoopman’s picture

maikelkoopman commented

Hi,

I understand your problems and I think we can improve this process of updating existing subthemes with new features that are added to socialblue.

The reason color and border radius are in social blue and not socialbase is that is prevent sites from duplicate css with overrides. With the upcoming release we have made better use of the 'core' color module. This is where the colors that can be changed via the theme settings by a site manager are now all put in a css file (brand.css). The color module can make a custom copy of of that css file with the new hex values and then we can cache the css file. This has better performance then what we do now with inline css in improved_theme_settings module.

Unfortunately this only works for colors, and not border-radius. If we can solve the problem of border radius in a performant way and we do not create specificity problems then we can change that as well. Please share your ideas and we can research them. I will start with the idea from slowflyer and see if can add these fields to socialbase already, so there is a better fallback.

kingdutch’s picture

kingdutch
he/him
Primary language English
commented
Status: Needs review » Needs work

The patch from #4 needs some work.

if ($card_radius != NULL) {

For all if statements we should prefer strict checking (e.g. $var !== NULL over $var != NULL).

We should check that the parsed in value is a proper integer if it's not null i.e. 'somestring' would pass the if statement but is not a valid value.

slowflyer’s picture

slowflyer commented

@Kingdutch: I tried strict checking. But when using strict checking, I still get the error.

maikelkoopman’s picture

maikelkoopman commented
Status: Needs work » Needs review
StatusFileSize
new theme_settings-2924676-11.patch2.23 KB

The Pull request to fix the warning is here: https://github.com/goalgorilla/open_social/pull/669. This is for 8.x-1.x-dev version

The patch in the attachment should work for 1.7 release.

I have also looked into inheritance from socialbase for custom subthemes. Because some things can be inherited, but a lot also can't (see https://www.drupal.org/docs/8/theming/sub-theme-inheritance), it will make it only more complicated to add "some" things to socialbase so it is activated in subthemes. I also think it is better to keep socialbase as clean as possible in terms of style, and just keep functionality in here.
If you have other ideas we can discuss further in individual feature issues.

slowflyer’s picture

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

#11 works fine

I agree, no unnecessary complicated stuff - keep it small and simple

jochemvn’s picture

jochemvn commented

Reviewed, a bit more simplified and merged. Will be a part of the 1.8 release of Open Social

jochemvn’s picture

jochemvn commented
Status: Reviewed & tested by the community » Fixed

  • maikelkoopman committed 27ec27b on 8.x-1.x 
    #2924676 by maikelkoopman,kingdutch: proper check if values exist before...
  • 65c8dd4 committed on 8.x-1.x
    Merge pull request #669 from goalgorilla/feature/2924676-theme-settings-...

Status: Fixed » Closed (fixed)

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