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 withdrawn notice to detailed guides #135

Merged
merged 8 commits into from Apr 25, 2016
Merged

Add withdrawn notice to detailed guides #135

merged 8 commits into from Apr 25, 2016

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Apr 22, 2016

See also alphagov/govuk-content-schemas#288

  • Extract withdraw presenter logic into module, use in case studies and detailed guides
  • Add withdrawn presenter and integration tests for both formats
  • Extract withdrawn template into re-usable partial
  • Extract withdrawn styles into re-usable helper mixin

Includes some minor bug fixes:

  • Fix margins on withdrawal notices
  • Avoid an empty "Part of" from showing in metadata component (example has no links)
  • Avoid an empty breadcrumb from showing (example has no parent)
  • Update titles to use average length, so they appear smaller and match what's currently live

Includes some refactoring:

  • Updates presenter tests to all extend PresenterTest – DRY up loading the content item example and presenting the example item, and allows tests to share helper methods

https://trello.com/c/P2CLsRtp/383-6-detailed-guides-migration-front-end-work-withdrawing-documents-small

Compare screenshot below with live example: https://www.gov.uk/guidance/eu-rules-on-the-use-of-chemicals

screen shot 2016-04-22 at 11 38 53

@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-135 Apr 22, 2016 Inactive
@fofr fofr force-pushed the withdrawable branch Apr 22, 2016
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-135 Apr 22, 2016 Inactive
@fofr
Copy link
Contributor Author

@fofr fofr commented Apr 22, 2016

Tests passing now alphagov/govuk-content-schemas#288 has been merged

@@ -20,10 +20,10 @@
<div class="column-two-thirds">
<%= render 'govuk_component/metadata',
from: @content_item.from,
part_of: @content_item.part_of,
other: {

This comment has been minimized.

@mgrassotti

mgrassotti Apr 22, 2016

when alphagov/static#774 will be merged then this could become:

<%= render 'govuk_component/metadata',
        from: @content_item.from,
        part_of: @content_item.part_of,
        first_published: @content_item.published,
        last_updated: @content_item.updated,
        see_updates_link: true
%>

right?

This comment has been minimized.

@boffbowsh

boffbowsh Apr 25, 2016
Contributor

I've merged that PR. Will this block be compatible?

This comment has been minimized.

@fofr

fofr Apr 25, 2016
Author Contributor

Updated and published are in other, so they are compatible.

@boffbowsh
Copy link
Contributor

@boffbowsh boffbowsh commented Apr 25, 2016

This needs rebasing

fofr added 8 commits Apr 21, 2016
Make withdrawn logic shareable between formats
* DRY up loading the content item example and presenting the example
item
* Allow all tests to share helper methods for shared features
* Update generator to use new test format
Detailed guides have longer titles.
The last paragraph still had its margin, which wasn’t collapsing
because of the container. When a `p` is the last child, remove the
margin, the notice has enough padding.
By not using the component `part_of`, we miss out on any logic
associated with it inside the component – eg don’t show if empty.

This also keeps all metadata consistently ordered between formats.

Keeping “from” and “part of” together makes sense too.
@fofr
Copy link
Contributor Author

@fofr fofr commented Apr 25, 2016

@boffbowsh Rebased.

@boffbowsh boffbowsh merged commit df29253 into master Apr 25, 2016
1 check passed
1 check passed
@govuk-ci
default Build #616 succeeded on Jenkins
Details
@boffbowsh boffbowsh deleted the withdrawable branch Apr 25, 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

3 participants