Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 36 additions & 12 deletions app/javascript/packs/personal-key-page-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,32 +41,56 @@ function unsetInvalidHTML() {
isInvalidForm = false;
}

function unsetEmptyResponse() {
input.setAttribute('aria-invalid', 'false');
input.classList.remove('margin-bottom-3');
input.classList.add('margin-bottom-6');
input.classList.remove('usa-input--error');

isInvalidForm = false;
}

function resetErrors() {
unsetEmptyResponse();
unsetInvalidHTML();
}

function resetForm() {
formEl.reset();
unsetInvalidHTML();
resetErrors();
}

function setEmptyResponse() {
input.setAttribute('aria-invalid', 'true');
input.classList.remove('margin-bottom-6');
input.classList.add('margin-bottom-3');
input.classList.add('usa-input--error');
input.focus();

isInvalidForm = true;
}

function handleSubmit(event) {
event.preventDefault();
resetErrors();

// As above, in case browser lacks HTML5 validation (e.g., IE < 11)
if (input.value.length < 19) {
setInvalidHTML();
if (input.value.length === 0) {
setEmptyResponse();
return;
}

const value = encodeInput(input.value);

if (value === personalKey) {
unsetInvalidHTML();
// Recovery code page, without js enabled, has a form submission that posts
// to the server with no body.
// Mimic that here.
formEl.removeEventListener('submit', handleSubmit);
formEl.submit();
} else {
if (input.value.length < 19 || value !== personalKey) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know we had the number 19 hardcoded before... is there a way to avoid hardcoding this magic number?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can make 19 a variable

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Meh it's probably not worth it because that doesn't address how we came up with 19 as a length

setInvalidHTML();
return;
}

// Recovery code page, without js enabled, has a form submission that posts
// to the server with no body.
// Mimic that here.
formEl.removeEventListener('submit', handleSubmit);
formEl.submit();
}

function show(event) {
Expand Down
9 changes: 6 additions & 3 deletions app/views/shared/_personal_key_confirmation_modal.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<div id="personal-key-confirm" class="display-none">
<%= render layout: '/shared/modal_layout' do %>
<%= render layout: 'shared/modal_layout' do %>
<div class="padding-y-8 padding-x-2 tablet:padding-x-8 cntnr-skinny border-box bg-white radius-lg key-badge">
<h2 class="margin-top-0 margin-bottom-2">
<%= t('forms.personal_key.title') %>
Expand All @@ -15,10 +15,13 @@
message: t('users.personal_key.confirmation_error'),
} %>
</div>
<form id="confirm-key" method="post" action="<%= update_path %>" name="key-confirm">
<form id="confirm-key" method="post" action="<%= update_path %>" name="key-confirm" novalidate="novalidate">
<%= render 'shared/personal_key_input', code: code %>
<%= hidden_field_tag :authenticity_token, form_authenticity_token %>

<%= render 'shared/validation_message', {
class: 'margin-bottom-2',
message: t('simple_form.required.text'),
} %>
<div class="clearfix margin-x-neg-2">
<div class="col col-12 sm-col-6 padding-x-2 margin-bottom-2 tablet:margin-bottom-0 tablet:display-none">
<button type="submit" class="usa-button usa-button--big usa-button--full-width personal-key-confirm">
Expand Down
23 changes: 23 additions & 0 deletions app/views/shared/_validation_message.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<%#
locals:
* message: error message to show
* class: classes to format inline error message
* text_tag: Optional override HTML tag for text content. Defaults to `p`.
%>
<%
classes = [
'usa-error-message',
'display-if-invalid',
'validation-message'
]
classes << local_assigns[:class] if local_assigns[:class]
message = local_assigns.fetch(:message, yield.presence)
text_tag = local_assigns.fetch(:text_tag, 'p')
%>

<%= tag.span(
role: 'alert',
class: classes,
) do %>
<%= content_tag(text_tag, message, class: 'usa-alert__text') %>
<% end %>
10 changes: 10 additions & 0 deletions spec/features/users/regenerate_personal_key_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@

click_acknowledge_personal_key
submit_form_without_entering_the_code
submit_form_with_the_wrong_code

expect(current_path).not_to eq account_path

Expand Down Expand Up @@ -142,4 +143,13 @@ def expect_to_be_back_on_manage_personal_key_page_with_continue_button_in_focus

def submit_form_without_entering_the_code
click_on t('forms.buttons.continue'), class: 'personal-key-confirm'
expect(page).to have_selector('.validation-message')
expect(page).not_to have_selector('#personal-key-alert')
end

def submit_form_with_the_wrong_code
fill_in 'personal_key', with: 'hellohellohello'
click_on t('forms.buttons.continue'), class: 'personal-key-confirm'
expect(page).to have_content(t('users.personal_key.confirmation_error'))
expect(page).not_to have_selector('.validation-message')
end