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

Add block variable for body attributes #285

Conversation

GreensterRox
Copy link

What does it do?

I've inheritied a project which relies on GA event tagging being driven by attributes dynamically added to the body tag.
Would you accept a modification which allows for a block variable being added to the body tag? This should be a non-breaking change for anyone not using this block var.

This change would allow uses of the template to specify custom attributes on the body tag if they so wish, the default would remain blank. We use it like this:

<body class="{% block body_classes %}{% endblock %}" {% block body_attributes %}{% endblock %}>

to render HTML like this:

<body class="noscript" data-journey="report-benefit-fraud:stage:details" >

What type of change is it?

New feature (non-breaking change which adds functionality)

@36degrees
Copy link
Contributor

Thanks for your PR!

Versioning is done separately to individual 'feature' pull requests – like this – so please revert your changes to CHANGELOG.md and lib/govuk_template/version.rb.

@GreensterRox
Copy link
Author

Hi @36degrees , thanks for your message. When I revert these two files do you still need me to squash into a single commit id ?

@36degrees
Copy link
Contributor

That'd be preferred, please. It just helps keep the repo's history clean.

@GreensterRox
Copy link
Author

GreensterRox commented Mar 23, 2017

I had to create another PR [ https://github.com//pull/286 ] because I can't seem to squash commits once a branch is pushed to the server - unless you know a way of doing this ?

@36degrees
Copy link
Contributor

What's the error message you're seeing? You'll need to force push as you're rewriting history.

@GreensterRox GreensterRox force-pushed the add-custom-body-attributes-block branch from bb40077 to 975db35 Compare March 23, 2017 14:34
@GreensterRox
Copy link
Author

I did force push and that seems to have done the trick :-)

@@ -13,7 +13,7 @@ You can [view a collection of auto-generated examples](http://alphagov.github.io

## Requirements

The Ruby language (1.9.3+), the build tool [Rake](http://rake.rubyforge.org/) & the dependancy management tool [Bundler](http://bundler.io/)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You’re not adding anything that requires Java though? Or is it already a requirement that’s been missed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @robinwhittleton, I noticed that 'webjar_packager.rb' attempts to create a jar file. That's why I added Java as a requirement. I had to install Java to make this work.

@fofr
Copy link
Contributor

fofr commented May 22, 2017

This PR seems to have stalled.

@GreensterRox do you still need this feature?

@36degrees Is there anything specific holding up this PR?

Is this a feature we want to add?

@GreensterRox
Copy link
Author

Hi @fofr, yes, this is indeed a feature that would help the project that I was previously working on, as it gives some extra flexibility by allowing custom attributes to be defined on the body tag.

@timpaul
Copy link
Contributor

timpaul commented Aug 30, 2018

Thank you for this work.

However, following the launch of the GOV.UK Design System, GOV.UK Template will now only get major bug fixes and security patches, so I'm going to close this.

We have captured the need for this feature in GOV.UK Frontend here.

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.

None yet

5 participants