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

Remove subscription links component #887

Merged
merged 3 commits into from
May 3, 2018

Conversation

andysellick
Copy link
Contributor

@andysellick andysellick commented Apr 30, 2018

The subscription links component has been moved from this app to the publishing components gem. This PR removes it and updates government-frontend to use the component from the gem.


Trello card: https://trello.com/c/NjgS3vym/55-modify-component-get-updates

Visual regression results:
https://government-frontend-pr-887.surge.sh/gallery.html

Component guide for this PR:
https://government-frontend-pr-887.herokuapp.com/component-guide/heading

@tijmenb tijmenb temporarily deployed to government-frontend-pr-887 April 30, 2018 11:27 Inactive
@boffbowsh boffbowsh temporarily deployed to government-frontend-pr-887 May 1, 2018 13:23 Inactive
@andysellick andysellick force-pushed the remove-subscription-links-component branch from 2f8faf2 to 0b2cf90 Compare May 1, 2018 15:28
@andysellick andysellick requested a review from vanitabarrett May 2, 2018 08:01
@andysellick andysellick changed the title [DO NOT MERGE] Remove subscription links component Remove subscription links component May 2, 2018
Copy link
Contributor

@sihugh sihugh left a comment

Choose a reason for hiding this comment

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

I checked this locally and against the PR adding the component to govuk_publishing_components. It looks sensible, renders correctly and removes the same things that were added in the other PR

Copy link
Contributor

@sihugh sihugh left a comment

Choose a reason for hiding this comment

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

Having though about this for a bit, I wonder whether we should leave the images in situ and remove in a subsequent PR, as people with cached CSS will continue to request them for a while afterwards?

Copy link
Contributor

@sihugh sihugh left a comment

Choose a reason for hiding this comment

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

We've checked it out on integration, and both things are still available so I withdraw my objections.

@andysellick andysellick merged commit 56c8e87 into master May 3, 2018
@andysellick andysellick deleted the remove-subscription-links-component branch May 3, 2018 12:33
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.

4 participants