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

Use print link component from govuk_publishing_components #1809

Merged
merged 2 commits into from
Jun 26, 2020

Conversation

theseanything
Copy link
Contributor

@theseanything theseanything commented Jun 26, 2020

This PR replaces the application component with the new component provided by govuk_publishing_components. Unused code for the application component is removed.

The components should be functional and visually equivalent.


Visual regression results:
before:
https://government-frontend.herokuapp.com/foreign-travel-advice/australia
https://government-frontend.herokuapp.com/universal-credit

after:
https://government-f-replace-pr-1cuoiy.herokuapp.com/foreign-travel-advice/australia
https://government-f-replace-pr-1cuoiy.herokuapp.com/universal-credit

@bevanloon bevanloon temporarily deployed to government-f-replace-pr-1cuoiy June 26, 2020 11:42 Inactive
@andysellick
Copy link
Contributor

Got any example URLs where the print link can be seen?

This replaces the app component with the new component provided by the
govuk_publishing_component. They should be functionally and visually
equivalent.
This has been replaced by the component in the
govuk_publishing_components. This code is no longer used.
@theseanything theseanything force-pushed the replace-print-link-component branch from 2da6b45 to e33453f Compare June 26, 2020 12:06
@bevanloon bevanloon temporarily deployed to government-f-replace-pr-1cuoiy June 26, 2020 12:06 Inactive
@theseanything
Copy link
Contributor Author

theseanything commented Jun 26, 2020

Got any example URLs where the print link can be seen?

@andysellick Added to the description :) Those are the only two content types that show print links.

Copy link
Contributor

@andysellick andysellick left a comment

Choose a reason for hiding this comment

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

Code looks fine. The component overlaps to the left on focus, but this was present before from the looks of it, so I don't think this is a blocking issue, but we might need to address at some point.

Screenshot 2020-06-26 at 13 50 38

Screenshot 2020-06-26 at 13 50 25

@theseanything theseanything merged commit 7737738 into master Jun 26, 2020
@theseanything theseanything deleted the replace-print-link-component branch June 26, 2020 13:22
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.

3 participants