LG-9973: Implement new default Sign In page from A/B test results#8542
LG-9973: Implement new default Sign In page from A/B test results#8542
Conversation
changelog: User-Facing Improvements, Sign In, Update layout of Sign In page to reflect results of recent A/B test
| track_event( | ||
| 'User Registration: enter email visited', | ||
| sign_in_a_b_test_bucket:, | ||
| from_sign_in:, |
There was a problem hiding this comment.
is this a useful param to keep at all?
There was a problem hiding this comment.
It could be, but my YAGNI hardliner instincts tell me we don't have an immediate use for it detached from the A/B test.
Generally, I do find that we're increasingly wanting to track "clicks" and correlating how a user arrives at a particular place, but maybe that's something we'd want to have some generic support for (e.g. a referrer base property on analytics events, or using ClickObserverComponent now that its implemented with sendBeacon as of @matthinz's improvements in #8496†).
† Previously, I would have avoided ClickObserverComponent for links which navigate the user away, since you couldn't trust that the analytics would be logged unless adding some async await + spinner button, often more trouble than it was worth.
There was a problem hiding this comment.
FWIW I think a referer property on events could be useful (perhaps normalized to just the path, could even leave it nil for links from other hosts if we wanted).
There was a problem hiding this comment.
(obviously we'd keep the misspelling from HTTP 😄 )
| ses_configuration_set_name: '' | ||
| set_remember_device_session_expiration: false | ||
| sign_up_mfa_selection_order_testing: '{ "default": 100, "authentication_app_priority": 0, "usability_priority": 0 }' | ||
| sign_in_a_b_testing: '{ "default": 100,"tabbed" :0, "banner": 0 } ' |
There was a problem hiding this comment.
😱 can't believe we had the extra space inside that string there, glad we're removing it
There was a problem hiding this comment.
😱 can't believe we had the extra space inside that string there, glad we're removing it
😅 Luckily it seemed to tolerate pretty well regardless!
|
I snuck in some styling improvements in d1f3066, to improve appearance in mobile:
I left a "TODO" to fill in with the upstream pull request, which I'll add before merging.
|
Done and reconciled with upstream uswds/uswds#5324 in 88d7f68. |
5 participants
🎫 Ticket
LG-9973
🛠 Summary of changes
Updates Sign In page to reflect the winner of the recent A/B test (the "tabbed" variant), removing all code associated with the A/B test and unused tests.
Easier to review with whitespace changes hidden: https://github.com/18F/identity-idp/pull/8542/files?w=1
📜 Testing Plan
👀 Screenshots