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

Upgrade govuk_publishing_components to 13.6.1 #1233

Merged
merged 3 commits into from
Feb 8, 2019

Conversation

karlbaker02
Copy link
Contributor

@karlbaker02 karlbaker02 commented Feb 8, 2019

This PR upgrades the govuk_publishing_components gem to version 13.6.1.


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

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

This commit upgrades the govuk_publishing_components gem to version `13.6.1`.
@benthorner benthorner temporarily deployed to government-frontend-pr-1233 February 8, 2019 10:00 Inactive
This commit fixes the assertions made in `choose_sign_in_test.rb` on the error-summary component provided by govuk_publishing_components. This component was upgraded in version `13.5.3` to use govuk-frontend styles, which in turn broke the assertions made here (see PR alphagov/govuk_publishing_components#692 for details).

Solo: @karlbaker02
@benthorner benthorner temporarily deployed to government-frontend-pr-1233 February 8, 2019 10:46 Inactive
@karlbaker02 karlbaker02 requested a review from alex-ju February 8, 2019 10:53
assert page.has_css?(".gem-c-error-summary")
assert page.has_css?(".gem-c-error-summary__title", text: 'You haven’t selected an option')
assert page.has_css?(".gem-c-error-summary__link[href='#option-0']", text: 'Please select an option')
assert page.has_css?(".govuk-error-summary")
Copy link
Contributor

Choose a reason for hiding this comment

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

These assertions were too tightly coupled to the component implementation in the gem. To avoid this happening again, perhaps we can just test for the existence of "You haven’t selected an option" on this page, without checking for the CSS generated by the gem (which should be internal).

alex-ju
alex-ju previously approved these changes Feb 8, 2019
Copy link
Contributor

@alex-ju alex-ju left a comment

Choose a reason for hiding this comment

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

This looks good, Karl. I agree with Tijmen that we may want to loosen up the tests to prevent this sort of updates on tests in future.

This commit updates the test `renders errors correctly` to make it less dependent on styling changes in `govuk_publishing_components` - this is how the tests broke when upgrading from `13.5.2` to `13.5.3`.
Copy link
Contributor

@alex-ju alex-ju left a comment

Choose a reason for hiding this comment

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

👍

@karlbaker02 karlbaker02 merged commit a0a3207 into master Feb 8, 2019
@karlbaker02 karlbaker02 deleted the upgrade-publishing-components-to-13-6-1 branch February 8, 2019 14:44
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