Skip to content

Lg 5417 mfa page design update#5989

Merged
SammySteiner merged 15 commits intomainfrom
LG-5417-MFA-page-design-update
Mar 4, 2022
Merged

Lg 5417 mfa page design update#5989
SammySteiner merged 15 commits intomainfrom
LG-5417-MFA-page-design-update

Conversation

@SammySteiner
Copy link
Contributor

@SammySteiner SammySteiner commented Feb 23, 2022

Why

Incorporate lessons learned from user testing MFA Setup page.
https://www.figma.com/file/ATLeAQQ6iY5CDG7X7LUfX6/LG-3948-Design%3A-Redesign-authentication-selection-page-from-create-account-page?node-id=1519%3A11550

  • updated text and translations
  • added icons
  • removed unnecessary and confusing content

changelog: Improvements, MFA Setup, Incorporating MFA Setup redesign (LG-5417)

@SammySteiner SammySteiner force-pushed the LG-5417-MFA-page-design-update branch from 4e63840 to edf6944 Compare February 23, 2022 19:10
<span class="usa-tag pin-bottom pin-right <%= option.disabled? ? 'bg-base-light' : 'bg-info-dark' %>">
<%= option.security_level %>
<%= image_tag(asset_url("mfa-options/#{option.type}.svg"), alt: "#{option.label} icon", class: 'usa-radio__image') %>
<div class="usa-radio__label--text tablet:padding-left-1"><%= option.label %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious about the tablet:padding-left-1 here. Should that be incorporated into the design system component? Is it accounting for whitespace from the HTML?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it should be incorporated into the design system component. Should I back it out here or release a patch of the design system?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I discussed with @jmdembe and we decided to keep it in here and address in the style guide another time.

Copy link
Contributor

Choose a reason for hiding this comment

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

I discussed with @jmdembe and we decided to keep it in here and address in the style guide another time.

Could you create a ticket for this to make sure it's tracked?

@aduth
Copy link
Contributor

aduth commented Feb 24, 2022

The vertical alignment of the icons looks a bit off to me at small screen sizes. Do you see this as well?

screenshot

@SammySteiner
Copy link
Contributor Author

SammySteiner commented Mar 3, 2022

The vertical alignment of the icons looks a bit off to me at small screen sizes. Do you see this as well?

screenshot

Yes! Thanks for that. Turns out the design system needs a few more adjustments. We discussed as a team and decided to make the changes here for now and create a separate ticket to group all these changes into the design system (and remove from here).

Should be fixed now. My sandbox is now up and running and can be looked at here: https://idp.ssteiner.identitysandbox.gov/

@mdiarra3 mdiarra3 requested a review from aduth March 4, 2022 14:23
@mdiarra3
Copy link
Contributor

mdiarra3 commented Mar 4, 2022

Huh why is the release step not running..

@SammySteiner SammySteiner force-pushed the LG-5417-MFA-page-design-update branch from 1959692 to c7dade4 Compare March 4, 2022 15:34
@SammySteiner SammySteiner merged commit 321eaeb into main Mar 4, 2022
@SammySteiner SammySteiner deleted the LG-5417-MFA-page-design-update branch March 4, 2022 15:44
aduth added a commit that referenced this pull request Jun 7, 2022
**Why**: Because they're unused since #5989

changelog: Internal, Code Quality, Remove unused code
aduth added a commit that referenced this pull request Jun 7, 2022
**Why**: Because they're unused since #5989

changelog: Internal, Code Quality, Remove unused code
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