Fix positioning of remember-this-browser checkbox#3769
Merged
Conversation
123456789012345678901234567890123456789012345678901234567890123456789012 **Why**: LG-1047 - chk + label are on diff. lines on some mobile devices **How**: Added flex container and leveraged several USWDS classes
Contributor
zachmargolis
left a comment
zachmargolis
left a comment
There was a problem hiding this comment.
LGTM -- just some code style changes to fix please
Also for next time for a visual regression like this, it helps to attach before/after screenshots!
| form_class: 'inline-block') %> | ||
| <div class="flex flex-row flex-align-center flex-wrap"> | ||
| <%= link_to(t('links.two_factor_authentication.get_another_code'), | ||
| otp_send_path(otp_delivery_selection_form: |
Contributor
There was a problem hiding this comment.
our style is to indent these arguments to match the indentation of the previous line, also might be easier to break the hash on to separate lines:
<%= link_to(t('links.two_factor_authentication.get_another_code'),
otp_send_path(otp_delivery_selection_form: {
otp_delivery_preference: @presenter.otp_delivery_preference,
resend: true
},
class: 'btn btn-link btn-border ico ico-refresh text-decoration-none flex-no-shrink margin-right-205',
form_class: 'inline-block') %>| t('forms.messages.remember_device'), | ||
| class: 'blue' %> | ||
| </inline-block> | ||
| t('forms.messages.remember_device'), |
Contributor
There was a problem hiding this comment.
ditto here about indentation
zachmargolis
suggested changes
May 14, 2020
app/views/two_factor_authentication/otp_verification/show.html.erb
Outdated
Show resolved
Hide resolved
**Why**: Indentation was out of style guidelines **How**: Indent additional lines pass the method name
zachmargolis
approved these changes
May 14, 2020
Contributor
zachmargolis
left a comment
zachmargolis
left a comment
There was a problem hiding this comment.
LGTM thanks for fixing
achapm
added a commit
that referenced
this pull request
May 20, 2020
* Update saml_idp revision * Lg 2939 update us gov banner (#3751) Add 'Here's how you know' section LG-2939 **Why**: So users can be confident in legitimacy/security of the site **How**: Base layout renders single ERB banner; JS enables toggling Co-authored-by: Nick Ng <nick.ng@gsa.gov> * [Snyk] Fix for 1 vulnerabilities (#3746) * fix: package.json & .snyk to reduce vulnerabilities The following vulnerabilities are fixed with a Snyk patch: - https://snyk.io/vuln/SNYK-JS-LODASH-567746 * Copy over .snyk too Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov> * Add spec to make sure that re-proofing costs behave as expected (#3754) * Fix spec to reflect the fact that we've removed SingleLogoutService * Revert "[Snyk] Fix for 1 vulnerabilities (#3746)" (#3755) This reverts commit 9323c2a. * fix bundler config to work (#3759) * Add deal and IAA information to service_providers table (LG-2865) (#3756) * Add deal and IAA information to service_providers table (LG-2865) **Why**: So that we can track metadata associated with SPs better * Various accessibility Fixes (#3761) * Fix TOTP setup nickname label (LG-2986) **Why**: Screen readers use this to describe the field, the old label was for a different field * Update accordion header's role to be a button (LG-2985) **Why**: It's intended to be clicked to show/hide information * Make the TOTP key copy button a button (LG-2987) **Why**: So it can be detected by screen readers as clickable * Make cancel link a button role on account deletion page (LG-2988) **Why**: Better screen reader support * Make sure hidden webauthn element is skipped by screen readers (LG-2984) (#3762) **Why**: Merely styling it as hidden, or setting aria-hidden does not hide sufficiently from screen readers, so we use the HTML5 "hidden" attribute to do this. * Update GSA logo alt text for screen readers (LG-2983) (#3763) **Why**: The alt text and link text both described GSA so now it's just the link text * LG-2959 Add a missing `return` statement when a Acuant SDK file is not permitted (#3765) **Why**: So the execution does not continue leading to a 500 error * Remove load testing scripts (#3764) **Why**: They were moved to their own repository (https://github.com/18f/identity-loadtest/) * Fix typo in method name (#3766) * Update order of elements in footer to match visual order (LG-2989) (#3768) * fix: package.json to reduce vulnerabilities (#3757) The following vulnerabilities are fixed with an upgrade: - https://snyk.io/vuln/SNYK-JS-YARGSPARSER-560381 * Remove the Acuant SDK from the desktop flow (#3767) **Why**: People using the desktop flow have to use a file picker already. Having the SDK there only means they have to wait for it to load. * LG-2375 Don't truncate the return to SP link (#3770) **Why**: SPs have names of varying length. Before this commit you can't read the name of SPs with long names. * Fix positioning of remember-this-browser checkbox (#3769) * Fix positioning of remember-this-browser checkbox **Why**: LG-1047 - chk + label are on diff. lines on some mobile devices **How**: Added flex container and leveraged several USWDS classes * Cleanup **Why**: To improve ERB template readability **How**: Better indentation; hash keys on multiple lines * Update yarn.lock (#3771) - To match package.json updates in #3757 * LG-2936 Add liveness check to main doc auth flow (#3758) * Hide/show email error messages via HTML5 hidden attribute (LG-2996) (#3773) **Why**: We got reports from an accessibility audit that IE 11 + JAWS was still reading these even though they're hidden by CSS * LG-260 Separate otp request tracking and throttling for verified phones (#2388) **Why**: So throttling a user with an unverified phone does not throttle a user with a verified phone and we can track these otp requests separately. **How**: Add a new phone_confirmed true/false column to the otp_requests_trackers. Add a new unique index for phone_fingerprint + phone_confirmed. Then drop the unique index for phone_fingerprint. Force any interaction with the table to use the new composite key. * Bring back i18n keys removed in #3767 (#3776) **Why**: They're still used * Account Delete: Require Password (LG-2964) (#3775) * Update account delete to require a password on the page (LG-2964) **Why**: To prevent drive-by deletions from leaving a screen unlocked * users/delete#show: slim to ERB conversion * Add analytics tracking Co-authored-by: Shade L. Jenifer <shade.jenifer@gsa.gov> Co-authored-by: Nick Ng <nick.ng@gsa.gov> Co-authored-by: Snyk bot <snyk-bot@snyk.io> Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov> Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> Co-authored-by: Tim Spencer <timothy.spencer@gsa.gov> Co-authored-by: Jonathan Hooper <jonathan.hooper@gsa.gov> Co-authored-by: Steve Urciuoli <steve.urciuoli@gsa.gov>
achapm
added a commit
that referenced
this pull request
May 20, 2020
* Update saml_idp revision * Lg 2939 update us gov banner (#3751) Add 'Here's how you know' section LG-2939 **Why**: So users can be confident in legitimacy/security of the site **How**: Base layout renders single ERB banner; JS enables toggling Co-authored-by: Nick Ng <nick.ng@gsa.gov> * [Snyk] Fix for 1 vulnerabilities (#3746) * fix: package.json & .snyk to reduce vulnerabilities The following vulnerabilities are fixed with a Snyk patch: - https://snyk.io/vuln/SNYK-JS-LODASH-567746 * Copy over .snyk too Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov> * Add spec to make sure that re-proofing costs behave as expected (#3754) * Fix spec to reflect the fact that we've removed SingleLogoutService * Revert "[Snyk] Fix for 1 vulnerabilities (#3746)" (#3755) This reverts commit 9323c2a. * fix bundler config to work (#3759) * Add deal and IAA information to service_providers table (LG-2865) (#3756) * Add deal and IAA information to service_providers table (LG-2865) **Why**: So that we can track metadata associated with SPs better * Various accessibility Fixes (#3761) * Fix TOTP setup nickname label (LG-2986) **Why**: Screen readers use this to describe the field, the old label was for a different field * Update accordion header's role to be a button (LG-2985) **Why**: It's intended to be clicked to show/hide information * Make the TOTP key copy button a button (LG-2987) **Why**: So it can be detected by screen readers as clickable * Make cancel link a button role on account deletion page (LG-2988) **Why**: Better screen reader support * Make sure hidden webauthn element is skipped by screen readers (LG-2984) (#3762) **Why**: Merely styling it as hidden, or setting aria-hidden does not hide sufficiently from screen readers, so we use the HTML5 "hidden" attribute to do this. * Update GSA logo alt text for screen readers (LG-2983) (#3763) **Why**: The alt text and link text both described GSA so now it's just the link text * LG-2959 Add a missing `return` statement when a Acuant SDK file is not permitted (#3765) **Why**: So the execution does not continue leading to a 500 error * Remove load testing scripts (#3764) **Why**: They were moved to their own repository (https://github.com/18f/identity-loadtest/) * Fix typo in method name (#3766) * Update order of elements in footer to match visual order (LG-2989) (#3768) * fix: package.json to reduce vulnerabilities (#3757) The following vulnerabilities are fixed with an upgrade: - https://snyk.io/vuln/SNYK-JS-YARGSPARSER-560381 * Remove the Acuant SDK from the desktop flow (#3767) **Why**: People using the desktop flow have to use a file picker already. Having the SDK there only means they have to wait for it to load. * LG-2375 Don't truncate the return to SP link (#3770) **Why**: SPs have names of varying length. Before this commit you can't read the name of SPs with long names. * Fix positioning of remember-this-browser checkbox (#3769) * Fix positioning of remember-this-browser checkbox **Why**: LG-1047 - chk + label are on diff. lines on some mobile devices **How**: Added flex container and leveraged several USWDS classes * Cleanup **Why**: To improve ERB template readability **How**: Better indentation; hash keys on multiple lines * Update yarn.lock (#3771) - To match package.json updates in #3757 * LG-2936 Add liveness check to main doc auth flow (#3758) * Hide/show email error messages via HTML5 hidden attribute (LG-2996) (#3773) **Why**: We got reports from an accessibility audit that IE 11 + JAWS was still reading these even though they're hidden by CSS * LG-260 Separate otp request tracking and throttling for verified phones (#2388) **Why**: So throttling a user with an unverified phone does not throttle a user with a verified phone and we can track these otp requests separately. **How**: Add a new phone_confirmed true/false column to the otp_requests_trackers. Add a new unique index for phone_fingerprint + phone_confirmed. Then drop the unique index for phone_fingerprint. Force any interaction with the table to use the new composite key. * Bring back i18n keys removed in #3767 (#3776) **Why**: They're still used * Account Delete: Require Password (LG-2964) (#3775) * Update account delete to require a password on the page (LG-2964) **Why**: To prevent drive-by deletions from leaving a screen unlocked * users/delete#show: slim to ERB conversion * Add analytics tracking Co-authored-by: Shade L. Jenifer <shade.jenifer@gsa.gov> Co-authored-by: Nick Ng <nick.ng@gsa.gov> Co-authored-by: Snyk bot <snyk-bot@snyk.io> Co-authored-by: Zach Margolis <zachary.margolis@gsa.gov> Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com> Co-authored-by: Tim Spencer <timothy.spencer@gsa.gov> Co-authored-by: Jonathan Hooper <jonathan.hooper@gsa.gov> Co-authored-by: Steve Urciuoli <steve.urciuoli@gsa.gov>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why: LG-1047 - chk + label are on diff. lines on some mobile devices
How: Added flex container and leveraged several USWDS classes