Hi,

First of all thanks for a great module, it works like a charm.
I do have a small feature request, it would be nice if the twig function 'site_setting_field(..)' was aware of the current language by default if no language code was provided.

Problem/Motivation

Some twig functions aren't aware of the current language right now.
Currently I have to inject the langcode to use it as follows:
{{ site_setting_field('[id]', '[field]', '[view_mode]', langcode) }}
Instead of ideally just:
{{ site_setting_field('[id]', '[field]') }}

That's because Drupal\site_settings\Twig\TwigExtension::singleSiteSettingsByNameAndField does the following:

if ($langcode && $site_setting->hasTranslation($langcode)) {
  $site_setting = $site_setting->getTranslation($langcode);
}

Proposed resolution

Let's make it so that if no langcode was provided, the current language id is given.
I suppose the above code snippet would have to become:

$langcode = $langcode ?? $this->languageManager->getCurrentLanguage()->getId();
if ($site_setting->hasTranslation($langcode)) {
  $site_setting = $site_setting->getTranslation($langcode);
}

(The language manager will need to be injected in the class still)

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

randalv created an issue. See original summary.

randalv’s picture

A little correction from my end, all the functions that call the "renderSiteSettings"-method in the extension class do seem to render with the correct language context, but the other two functions (`site_setting_field` && `site_setting_entity_by_name`) specifically check for a langcode and load that translation, so those should be altered I think.

I'll add a MR for it.

randalv’s picture

Status: Active » Needs review

Merge request added!

I also took the liberty to remove what I assume was an orphaned `var_dump` 😇

randalv’s picture

Title: Twig functions aren't aware of the currently active language » Some twig functions aren't aware of the currently active language
Issue summary: View changes
scott_euser’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Ha good spot on the var_dump, thank you!

Let me know if you're able to add automated test coverage to this? I believe the sample data module would need some translations added in order to make it testable. If you aren't able I'll add to my list to eventually do.

Thanks!

svendecabooter made their first commit to this issue’s fork.

svendecabooter’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests

I have added test coverage for this.
Not sure if this belongs in LoaderLanguageTest specifically, but it had all the setup logic already available, so seemed like the easiest place to add this test.

scott_euser’s picture

Status: Needs review » Reviewed & tested by the community

Cheers thank you! Added a post update hook to clear the cache for the service args change just in case per https://www.drupal.org/project/drupal/issues/2743313

Merging, thanks!

  • scott_euser committed 3e45f52e on 2.0.x authored by randalv
    Issue #3507349 by svendecabooter, randalv: Some twig functions aren't...
scott_euser’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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

svendecabooter’s picture

Could a new release be tagged with this improvement?

scott_euser’s picture

Sure done