Skip to content

LG-6387: Make CTA button more prominent#6986

Merged
jmdembe merged 18 commits intomainfrom
LG-6387-make-cta-more-prominent
Sep 23, 2022
Merged

LG-6387: Make CTA button more prominent#6986
jmdembe merged 18 commits intomainfrom
LG-6387-make-cta-more-prominent

Conversation

@jmdembe
Copy link
Contributor

@jmdembe jmdembe commented Sep 19, 2022

This PR is looking to decrease confusion of where the "Create your account" button is located.

Context:

  • The issue has been raised that some partner sites direct you to the Login.gov front-door to click "Create an account" after clicking "Create a new account" on their respective site, requiring users to do the same action twice.

  • The secure.login.gov IdP landing page features username and password fields for users to sign in and create an account. There have been notes from our partners and agencies like CA.gov who have expressed concerns about the visibility of the “Create account” CTA within the Login.gov “front-door” sign-in landing page.

Proposed solution: Improve layout of the front door sign-in page to reduce user confusion of options to select

before Figma design after
Image of current login page for secure.login.gov Image of proposed design change for secure.login.gov in Figma Image of proposed change for secure.login.gov in the browser

@jmdembe jmdembe force-pushed the LG-6387-make-cta-more-prominent branch from b0b9f24 to 6738215 Compare September 19, 2022 20:39
@jmdembe jmdembe requested review from a team and aduth September 20, 2022 13:11
@jmdembe jmdembe force-pushed the LG-6387-make-cta-more-prominent branch 3 times, most recently from e9d0d9a to 1eddbf3 Compare September 20, 2022 14:54
@jmdembe jmdembe force-pushed the LG-6387-make-cta-more-prominent branch from 1eddbf3 to 825671e Compare September 20, 2022 17:35
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

<%= f.submit t('links.next'), full_width: true, big: false, wide: false, class: 'margin-bottom-2' %>
<%= f.submit t('links.next'), full_width: true, big: false, wide: false, class: 'margin-bottom-3' %>
<h2 class='margin-top-1 margin-bottom-3 text-normal text-center text-center--line text-bold border-bottom'>
<span class='padding-left-1 padding-right-1 bg-white' ><%= t('headings.create_account_with_sp.cta', app_name: APP_NAME) %></span>
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we could use the -x- styles for left + right?

Suggested change
<span class='padding-left-1 padding-right-1 bg-white' ><%= t('headings.create_account_with_sp.cta', app_name: APP_NAME) %></span>
<span class='padding-x-1 bg-white' ><%= t('headings.create_account_with_sp.cta', app_name: APP_NAME) %></span>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah this is a lot better. thanks!

<div class='margin-bottom-4'>
<%= f.submit t('links.next'), full_width: true, big: false, wide: false, class: 'margin-bottom-2' %>
<%= f.submit t('links.next'), full_width: true, big: false, wide: false, class: 'margin-bottom-3' %>
<h2 class='margin-top-1 margin-bottom-3 text-normal text-center text-center--line text-bold border-bottom'>
Copy link
Contributor

@aduth aduth Sep 20, 2022

Choose a reason for hiding this comment

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

Minor: I saw some commits which moved styles from the CSS to utility classes. I might actually be more in favor of the opposite, so that we can treat this more as a self-contained component.

e.g.

<h2 class="separator-heading">
  <span class="separator-heading__text">
    <%= t('headings.create_account_with_sp.cta', app_name: APP_NAME) %>
  </span>
</h2>

<div class='margin-bottom-4'>
<%= f.submit t('links.next'), full_width: true, big: false, wide: false, class: 'margin-bottom-2' %>
<%= f.submit t('links.next'), full_width: true, big: false, wide: false, class: 'margin-bottom-3' %>
<h2 class='margin-top-1 margin-bottom-3 text-normal text-center text-center--line text-bold border-bottom'>
Copy link
Contributor

Choose a reason for hiding this comment

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

This has both text-normal and text-bold classes. Based on the Figma design, I'm assuming it should only be text-bold.

Comment on lines +103 to +106
.text-center--line {
border-bottom-color: color('primary-light');
line-height: 0.1em;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Couple discrepancies I'm noticing between the implementation and Figma reference:

  1. Font size: 22px vs. 18px in the Figma
  2. Padding: 0.5rem vs. 1rem in the Figma
Suggested change
.text-center--line {
border-bottom-color: color('primary-light');
line-height: 0.1em;
}
.text-center--line {
border-bottom-color: color('primary-light');
line-height: 0.1em;
font-size: 1.125rem;
span {
@include u-padding-x(2);
}
}


.text-center--line {
border-bottom-color: color('primary-light');
line-height: 0.1em;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is 0.1em significant? Would it make any difference if it were 0 (collapse all line-height) or 1px (match the size of the border)?


.text-center--line {
border-bottom-color: color('primary-light');
line-height: 0.1em;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm observing that at small phone sizes (e.g. iPhone SE) where text wrapping occurs, the small line-height causes the text to collapse on itself in French and Spanish versions.

image

image

@jmdembe jmdembe force-pushed the LG-6387-make-cta-more-prominent branch from 94603d1 to 5a541dc Compare September 22, 2022 20:40
@jmdembe jmdembe merged commit b0b171d into main Sep 23, 2022
@jmdembe jmdembe deleted the LG-6387-make-cta-more-prominent branch September 23, 2022 19:53
@solipet solipet mentioned this pull request Sep 26, 2022
@aduth aduth mentioned this pull request Sep 29, 2022
1 task
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