- Issue created by @slowflyer
- π©πͺGermany donquixote
I don't like `!= null`. It obfuscates the type conversions. See https://3v4l.org/fL8Dp
E.g. `'' == null`, but `'' !== null`.Either we only want to detect NULL, then we use `=== NULL` or `!== NULL`.
Or we want to detect false-ish values, then we could use `if ($v)` or `if (!$v)` or `if ((bool) $v)`.
(We could also use `empty()` but this hides undefined variables.)Also NULL would be uppercase with Drupal coding standards. And there is a missing space after first `if`.
As far as I see nad know a setting border_radius does not exits in socialbase or socailblue and should be replaced by "card_radius"
Then other instances of 'border_radius' also need to be replaced.
E.g. here in the same function social_swiftmail_preprocess_swiftmailer():$variables['border_radius'] = $border_radius;
As far as I can see, setting this in variables will have no effect. But then again, I don't see either border_radius nor card_radius in the swiftmailer template. Maybe I am missing something?
----
I see one place where 'card_radius' is used when generating in-page CSS, in `improved_theme_settings_page_attachments()`.
But this has nothing to do with swiftmailer. - π©πͺGermany donquixote
Btw a shortcut would be this:
$border_radius = Xss::filter(theme_get_setting('border_radius', $theme_id) ?? '0');
It would mean that the Xss::filter() is still executed even for zero, but I think it is acceptable.
I am putting here '0' (string) and not 0 (integer) because the filter function will convert it to string anyway.But perhaps all of this is just a leftover that needs to be cleaned up?
- π©πͺGermany donquixote
For the `$disabled_greeting_keys`:
- The stored setting could be NULL or '' (empty string). Both should have the same effect.
- The stored setting could have trailing line breaks or even trailing spaces in line endings. In that case, or in case the setting itself is an empty string (as above), the exploded array would contain an empty string item.
I assume in either case we want to ignore these empty string parts. - The `$message['key']` might also be an empty string.
In that case, I assume we want to treat it the same as if it is NULL, so that the heading is always shown.
I say all of this from looking at the code, I did not actually test anything.
Btw the control flow in this part of the function is a bit cluttered and redundant, and the different parts are not in an ideal order.
It could be refactored like this:// Try to add a heading message. $display_name = $user?->getDisplayName() ?? $message['params']['context']['display_name'] ?? NULL; if (!$display_name) { return; } if (!empty($message['key'])) { $disabled_greeting_keys = $social_swiftmail_config->get("disabled_user_greeting_keys") ?? ''; $disabled_greeting_keys = explode("\r\n", $disabled_greeting_keys); if (in_array($message['key'], $disabled_greeting_keys)) { return; } } $variables['heading'] = t( 'Hi <strong>@display_name</strong>', ['@display_name' => $display_name], ['langcode' => $message['langcode']], );
Note that:
- I appended `?? ''` to to
$social_swiftmail_config->get("disabled_user_greeting_keys") ?? ''
. - I replaced the
isset($message['key'])
with!empty($message['key'])
to also cover empty string. But we have to look the possible empty-ish values this might have, and what the consequences of that would be.
- πΊπ¦Ukraine kashandarash
hello, there is PR for this with two border-radius https://github.com/goalgorilla/open_social/pull/3482/files
- Status changed to Needs review
about 1 year ago 10:42am 18 August 2023 - Status changed to Needs work
about 1 year ago 3:31pm 20 August 2023 - πΊπΈUnited States SocialNicheGuru
Uploaded a patch based on PR that applies to 11.9.x
- Status changed to Fixed
about 1 year ago 7:50am 21 August 2023 Automatically closed - issue fixed for 2 weeks with no activity.