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

Fix display of contact details on policy groups #131

Merged
merged 3 commits into from Apr 4, 2016
Merged

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Apr 1, 2016

Don't show contact in contents when no email

When an email is excluded it’s sent as an empty string in the content item, rather than being omitted. The template checks for email.present? but the contents logic doesn’t, which creates a discrepancy.

screen shot 2016-04-01 at 14 27 25

Use single govspeak component

When using separate components the vertical rhythm of content could break. We apply special rules to the first element in govspeak, eg removing headings from the first H2.

The second govspeak component would remove the margin from the “Contact details” heading, so its content would appear closer to the body than it should.

Rather than wrap that content and give it some margin, concatenate the two bodies and pass to a single component.

Before

screen shot 2016-04-01 at 14 27 11

After

screen shot 2016-04-01 at 14 26 55

@boffbowsh boffbowsh deployed to government-frontend-pr-131 Apr 1, 2016 Active
@fofr
Copy link
Contributor Author

@fofr fofr commented Apr 1, 2016

Test failures appear unrelated: ServiceManualTopicTest cc @AndrewVos

@boffbowsh boffbowsh deployed to government-frontend-pr-131 Apr 1, 2016 Active
@AndrewVos
Copy link
Contributor

@AndrewVos AndrewVos commented Apr 4, 2016

@fofr Fixed in #130, apologies, turns out a govuk-content-schema PR shouldn't have been merged without that one.

1 similar comment
@AndrewVos
Copy link
Contributor

@AndrewVos AndrewVos commented Apr 4, 2016

@fofr Fixed in #130, apologies, turns out a govuk-content-schema PR shouldn't have been merged without that one.

fofr added 3 commits Apr 1, 2016
When an email is excluded it’s sent as an empty string in the content
item, rather than being omitted. The template checks for
`email.present?` but the contents logic doesn’t, which creates a
discrepancy.
When using separate components the vertical rhythm of content could
break. We apply special rules to the first element in govspeak, eg
removing headings from the first H2.

The second govspeak component would remove the margin from the “Contact
details” heading, so its content would appear closer to the body than
it should.

Rather than wrap that content and give it some margin, concatenate the
two bodies and pass to a single component.
The `offset-one-third` class was included, but the styles for it were
not available on the working groups format.
@fofr fofr force-pushed the contact-policy-groups branch to 864317a Apr 4, 2016
@boffbowsh boffbowsh deployed to government-frontend-pr-131 Apr 4, 2016 Active
@fofr
Copy link
Contributor Author

@fofr fofr commented Apr 4, 2016

Tests are passing now.

@tvararu
Copy link
Contributor

@tvararu tvararu commented Apr 4, 2016

👍

@tvararu tvararu merged commit d5957dd into master Apr 4, 2016
1 check passed
1 check passed
@govuk-ci
default Build #584 succeeded on Jenkins
Details
@tvararu tvararu deleted the contact-policy-groups branch Apr 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants