- Issue created by @tanc
- @tanc opened merge request.
- Status changed to Needs review
about 1 year ago 7:50am 7 November 2023 - 🇮🇹Italy tanc Italy
One thing I haven't considered is that I think you can have multiple entities for a single site setting. So maybe this function should load and render all of them associated to that machine name?
- Status changed to Needs work
about 1 year ago 8:28am 7 November 2023 - 🇬🇧United Kingdom scott_euser
Thanks for the work on this! Good idea!
Other things that we should do are:
- Add to the SiteSettingsFullLoaderTwigExtensionTest test
- Update README.md with example of this
- (Post-merge) Update module homepage with example of this
- 🇬🇧United Kingdom scott_euser
One thing I haven't considered is that I think you can have multiple entities for a single site setting. So maybe this function should load and render all of them associated to that machine name?
Yes I suppose either we always return an array of site setting entities, or only an array if the type supports multiple. Perhaps the latter would be more in line with what developers might expect?
- 🇮🇹Italy tanc Italy
Thanks for the feedback @scott_euser
I've updated it to make things a bit simpler and just return an array of all associated entities for a given site setting. I think this approach is easier to understand and fits better with the current code (changes less). I've updated the MR.
If you're happy with this approach I'll write a test and update the README
- Status changed to Needs review
about 1 year ago 9:45am 7 November 2023 -
scott_euser →
committed 8c0f84ca on 2.0.x authored by
tanc →
Issue #3399761 by tanc: Add twig function to render site setting by...
-
scott_euser →
committed 8c0f84ca on 2.0.x authored by
tanc →
- Status changed to Fixed
about 1 year ago 12:39pm 7 November 2023 Automatically closed - issue fixed for 2 weeks with no activity.