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

Move meta description and page class to base layout #230

Merged
merged 3 commits into from Jan 16, 2017
Merged

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Jan 12, 2017

When adding formats it's easy to forget to set the meta description to the content item’s description. This is a sensible default. Description isn't always present (eg html publications), in those cases omit the meta tag.

Almost all formats set the page class consistently to the dasherized version of the format name. Make those that were slightly different consistent and switch responsibility to the base layout, DRYing up the format templates. This reduces the risk of a format being accidentally added with a different page_class behaviour.

fofr added 3 commits Jan 5, 2017
* Simplify each of the formats
* Protect against formats forgetting to include
Always set to the singular dasherized format
All formats set the page_class consistently, we can move this directly
into the base layout and DRY up the individual format templates.

This reduces the risk that a format is added in a way that behaves
differently.
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-230 Jan 12, 2017 Inactive
@fofr fofr changed the title [DISCUSS] Move meta description and page class to base layout Move meta description and page class to base layout Jan 15, 2017
@gpeng
gpeng approved these changes Jan 16, 2017
@fofr fofr merged commit 67e5d36 into master Jan 16, 2017
1 check passed
1 check passed
continuous-integration/jenkins/branch This commit looks good
Details
@fofr fofr deleted the refactor-meta-tags branch Jan 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.