HUH-74: Verify sign-in hint on interstitial page - live test #1585
Conversation
This is a time-limited live test of showing a Verify hint to the users who have verified with GOV.UK Verify before. This test should be teared down before 20th December 2019. A few points to consider when reviewing: - the hint will only show on personal tax account sign-in page - CSP rules have been adjusted by adding a new domain to our S3 where we store IDP and Verify logos. It's an identical copy of the gov_uk_app_config gem - added tests to confirm correct behaviour of the hint show logic
app/views/content_items/service_sign_in/_choose_sign_in.html.erb
Outdated
Show resolved
Hide resolved
And added click tracking.
|
There was a problem hiding this comment.
Thanks for raising this @jakubmiarka. Really interested to see this being rolled out and the results. Looks in a good shape overall, I left a few frontend comments.
| .verify-hint-logos { | ||
| margin-bottom: 15px; | ||
|
|
||
| .verify-hint-logos-idp { |
There was a problem hiding this comment.
We should use BEM for class names (e.g. verify-hint-box, verify-hint-box__logos, verify-hint-box__logo-idp, .verify-hint-box__logo-verify) which will make SCSS nesting unnecessary.
| @@ -27,6 +27,7 @@ | |||
| <%= render "govuk_publishing_components/components/fieldset", legend_text: legend_text do %> | |||
| <div class="govuk-grid-row"> | |||
| <div class="govuk-grid-column-two-thirds"> | |||
| <div id="verify-hint" data-module="show-gov-uk-verify-hint" style="display:none"></div> | |||
There was a problem hiding this comment.
We should avoid in-line styling. Can we rely on CSS and .js-enabled class to hide this?
Also I don't see where we need an id for this element.
There was a problem hiding this comment.
As much as I hate in-inline styling I have resorted to this as I don't want to make the element (even empty) visible. It's not only for people with no-js (so .js-enabled class does not help) but people who haven't used Verify successfully before shouldn't have get the element visible...
The ID is a leftover from original implentation, it didn't get used. I'll remove it.
- move the URL definition on which the script to run further up - added alt text to the logos - removed a redundant ID
|
@andysellick and @alex-ju thanks for having a look at the PR and your comments. I've addressed in the latest commit or responded to them. Could you have another look and review (and approve) since we only have less than 4 days to run this test. Thanks! |
This is a time-limited live test of showing a Verify hint to the users
who have verified with GOV.UK Verify before.
This test should be teared down before 20th December 2019.
The way this works, is on page load, an Ajax call is made to Verify domain to check whether
the user has successfully verified with an IDP before, if they haven't nothing happens. If they
have a new box is displayed at the top of the page showing their IDP and an option to continue.
A few points to consider when reviewing:
IDP and Verify logos. It's an identical copy of the gov_uk_app_config gem (had to add and exclusion to robocop as it's failing the linter)
Screenshot:
Visual regression results:
https://government-frontend-pr-1585.surge.sh/gallery.html
Component guide for this PR:
https://government-frontend-pr-1585.herokuapp.com/component-guide