Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Makes page header block lede rendering slightly more flexible #492

Conversation

ctorgalson
Copy link
Contributor

  • Retains current rendering, but anticipates that the {{ lede }} render array could have a different structure.
  • A forthcoming change to localgovdrupal/localgov_guides may require this.
  • See: 122 consider stacked h1 pattern for guides localgov_guides#133
  • Re: 491-localgov_base-directly-access-value-property-of-lede-in-localgov-page-header-blockhtmltwig

- Retains current rendering, but anticipates that the {{ lede }} render array
  could have a different structure.
- A forthcoming change to localgovdrupal/localgov_guides may require this.
- See: localgovdrupal/localgov_guides#133
- Re: 491-localgov_base-directly-access-value-property-of-lede-in-localgov-page-header-blockhtmltwig
@finnlewis
Copy link
Member

Thanks for this @ctorgalson

Discussing in Merge Monday and think we need some input from Mark and perhaps look for consistency in the approach between guides, step-by-step and publications.

In localgov_publications we've gone the NHS route: https://github.com/localgovdrupal/localgov_publications/blob/1.x/templates/localgov-publication-page-header-block.html.twig#L20-L31

Be interested to hear from @markconroy on this too.

@andybroomfield
Copy link
Contributor

Ref, also maybe a third option localgovdrupal/localgov_core#192

@markconroy
Copy link
Member

I'm not fully sure of the problem being solved, but from a code perspective this looks "good enough for now; safe enough to try". We can create a follow up if we want/need to so that we have a unified approach to these type things.

@ctorgalson
Copy link
Contributor Author

@markconroy The idea is to make localgov_base less reliant on a render array with a specific structure.

Changes to localgov_guides may change the array, but it's already possible for any sub-theme or module to preprocess lede into something that doesn't have a #value property.

@finnlewis
Copy link
Member

Just discussing in Merge Monday.

Andy mentioned localgovdrupal/localgov_core#193 which I see you have comment on @ctorgalson

I also see localgovdrupal/localgov_guides#133 (comment)

So.... actually this makes sens for now. @markconroy is happy to merge this for now, making the $lede variable more flexible in localgov _base.

@finnlewis finnlewis merged commit 2f5a65f into 1.x Nov 13, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

localgov_base directly access #value property of lede in localgov-page-header-block.html.twig
4 participants