Conversation
There was a problem hiding this comment.
By changing the link into a button, the button loses its border when you hover over it, and I wasn't sure how to fix that aside from commenting this out. Let me know if there's a better way to do that.
There was a problem hiding this comment.
Why didn't we need this for the other uses of button_to in the app? Unsure if we need it
monfresh
force-pushed
the
mb-update-personal-key-ui
branch
from
1adf8d7 to
718ccef
Compare
| / = link_to manage_personal_key_path do | ||
| / span.hide | ||
| / = t('account.items.personal_key') | ||
| / = t('account.links.regenerate_personal_key') |
There was a problem hiding this comment.
The original link had a hidden span, presumably for accessibility purposes. I wasn't sure if we needed it now that we're using button_to. If we do need it, where would it go?
There was a problem hiding this comment.
Looks like button_to can take a block and we can put the span.hide inside of that: https://github.com/18F/identity-idp/blob/master/app/views/accounts/actions/_disable_totp.html.slim#L2
zachmargolis
left a comment
zachmargolis
left a comment
There was a problem hiding this comment.
LGTM after we fix the span.hide bit to match existing
| / = link_to manage_personal_key_path do | ||
| / span.hide | ||
| / = t('account.items.personal_key') | ||
| / = t('account.links.regenerate_personal_key') |
There was a problem hiding this comment.
Looks like button_to can take a block and we can put the span.hide inside of that: https://github.com/18F/identity-idp/blob/master/app/views/accounts/actions/_disable_totp.html.slim#L2
There was a problem hiding this comment.
Why didn't we need this for the other uses of button_to in the app? Unsure if we need it
monfresh
force-pushed
the
mb-update-personal-key-ui
branch
3 times, most recently
from
1cef1d4 to
341d131
Compare
**Why**: For a better user experience.
|
I updated to add the |
2 participants
Why: For a better user experience.