Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MHV-61146] sets focus on error field after clicking send btn #31543

Draft
wants to merge 54 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
da13cf0
[MHV-61146] sets focus on error field after clicking send btn
vsaleem Aug 22, 2024
b870566
uncomment failed tests
fazilqa Aug 22, 2024
9368710
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Aug 23, 2024
e38da14
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Aug 26, 2024
b9020c1
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Aug 26, 2024
d9383ae
update verifyFocusOnErrorMessage
fazilqa Aug 26, 2024
b796b4a
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Aug 27, 2024
a86a9e7
[MHV-61146] Resolve recipient select bug, user is not allowed to send…
vsaleem Aug 28, 2024
1ba0d15
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Aug 28, 2024
9be8af8
[MHV-61146] Bug fix, prevents user from saving or sending a message …
vsaleem Aug 28, 2024
9270afa
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Aug 28, 2024
c21b07f
add test to compose-errors spec
fazilqa Aug 28, 2024
39704e6
Merge remote-tracking branch 'origin/61146-Resolve-focus-on-validatio…
fazilqa Aug 28, 2024
f785e06
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
fazilqa Aug 28, 2024
205e5bf
[MHV-61146] Accessibility fix to adjust focus to announce the recipie…
vsaleem Aug 30, 2024
695d1f4
[MHV-61146] Unit test fixes in progress
vsaleem Aug 30, 2024
3b427a7
test fix
fazilqa Sep 4, 2024
dc3c4f7
[MHV-61146] unit test updates for electronic signature checkbox
vsaleem Sep 4, 2024
a6cc584
cleanup
vsaleem Sep 5, 2024
48ce670
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Sep 5, 2024
741ca2f
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Sep 6, 2024
56e7e6f
cleanup
vsaleem Sep 6, 2024
ace4e23
fix failed tests
fazilqa Sep 6, 2024
daaa373
code cleanup
fazilqa Sep 6, 2024
f1a6792
update test
fazilqa Sep 9, 2024
19cf3ff
cleanup
vsaleem Sep 10, 2024
272deeb
[MHV-61146] cleanup; Removed separate signature and checkbox validati…
vsaleem Sep 10, 2024
183626e
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Sep 16, 2024
f19a1a9
[MHV-61146] Resolve bug; focus remains on recipient after selection a…
vsaleem Sep 17, 2024
2d180a9
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Sep 17, 2024
e2e6719
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Sep 17, 2024
8fbf8ce
[MHV-61146] Remove aria-live polite from alert to prevent announcing …
vsaleem Sep 17, 2024
f66f488
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Sep 18, 2024
2b7aa8a
update method according to correct behavior
fazilqa Sep 19, 2024
4b5cfe7
[MHV-61146] Bug fix, user can save draft with attachment modal
vsaleem Sep 24, 2024
82aefb7
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Sep 24, 2024
c47aa28
[MHV-61146] Cleanup unit tests and ElectronicSignature fields must be…
vsaleem Sep 24, 2024
4c90da2
[MHV-61146] cleanup
vsaleem Sep 25, 2024
886837e
cleanup
vsaleem Sep 25, 2024
3c9a31c
update focus assertion
fazilqa Sep 25, 2024
4307e74
update constants
fazilqa Sep 25, 2024
273345a
Merge remote-tracking branch 'origin/61146-Resolve-focus-on-validatio…
fazilqa Sep 25, 2024
64589e7
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Sep 26, 2024
19c4ab6
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Sep 26, 2024
8c86d74
checking for interpolation errors
vsaleem Sep 27, 2024
08dc628
[MHV-61146] Adding cy test updates
vsaleem Sep 30, 2024
d30e51d
Merge branch 'main' into 61146-Resolve-validation-focus-3rd-branch
vsaleem Sep 30, 2024
192208a
Merge branch 'main' into 61146-Resolve-validation-focus-3rd-branch
vsaleem Sep 30, 2024
379ee65
[MHV-61146] Fix validation focus on Reply page
vsaleem Sep 30, 2024
7e16866
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Oct 1, 2024
c49bf05
Merge branch '61146-Resolve-validation-focus-3rd-branch' into 61146-R…
vsaleem Oct 1, 2024
ad69100
Update secure-messaging-keyboard-nav-save-draft.cypress.spec.js
vsaleem Oct 1, 2024
01dda73
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Oct 4, 2024
ae53f51
Merge branch 'main' into 61146-Resolve-focus-on-validation-errs-sm
vsaleem Oct 4, 2024
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

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const ElectronicSignature = ({
ElectronicSignature.propTypes = {
checkboxError: PropTypes.string,
checked: PropTypes.bool,
electronicSignature: PropTypes.string,
nameError: PropTypes.string,
onCheckboxCheck: PropTypes.func,
onInput: PropTypes.func,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,21 @@ const RecipientsSelect = ({

const handleRecipientSelect = useCallback(
e => {
const recipient = recipientsList.find(r => +r.id === +e.detail.value);
setSelectedRecipient(recipient);
if (recipient.signatureRequired || isSignatureRequired) {
const recipient = recipientsList?.find(r => +r.id === +e.detail.value);

if (e.detail.value === '') {
setSelectedRecipient({});
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious why do we need to set this to a blank object {} if normal values are strings? should it be null or ' ' ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having null or an ' ' here caused a small bug that would not focus on the select field when the value was deselected. We need the blank {} here because selectedRecipient and recipient are returning objects when selected. If e.detail.value has an empty value, ' ', then the selectedRecipient should be an empty {}.

Copy link
Contributor

Choose a reason for hiding this comment

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

understood. As as we tested during code pairing, we did ensure that selectedRecipient is populated with an object, so that makes sense

}

if (e.detail.value !== '') {
setSelectedRecipient(recipient);
}

if (recipient?.signatureRequired || isSignatureRequired) {
setAlertDisplayed(true);
}
},
[recipientsList, isSignatureRequired],
[recipientsList, isSignatureRequired, setSelectedRecipient],
);

return (
Expand All @@ -94,7 +102,7 @@ const RecipientsSelect = ({
id="recipient-dropdown"
label="To"
name="to"
value={defaultValue !== undefined ? defaultValue : ''}
value={defaultValue}
onVaSelect={handleRecipientSelect}
class="composeSelect"
data-testid="compose-recipient-select"
Expand All @@ -111,8 +119,6 @@ const RecipientsSelect = ({
</VaSelect>
{alertDisplayed && (
<VaAlert
role="alert"
aria-live="polite"
ref={alertRef}
class="vads-u-margin-y--2"
closeBtnAriaLabel="Close notification"
Expand All @@ -124,7 +130,7 @@ const RecipientsSelect = ({
visible
data-testid="signature-alert"
>
<p className="vads-u-margin-y--0">
<p className="vads-u-margin-y--0" role="alert" aria-live="polite">
{isSignatureRequired === true
? Prompts.Compose.SIGNATURE_REQUIRED
: Prompts.Compose.SIGNATURE_NOT_REQUIRED}
Expand All @@ -141,6 +147,8 @@ RecipientsSelect.propTypes = {
defaultValue: PropTypes.number,
error: PropTypes.string,
isSignatureRequired: PropTypes.bool,
setCheckboxMarked: PropTypes.func,
setElectronicSignature: PropTypes.func,
};

export default RecipientsSelect;
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@
noTimeout();
}
},
[draft, messageBody, attachments],

Check warning on line 137 in src/applications/mhv-secure-messaging/components/ComposeForm/ReplyDraftItem.jsx

View workflow job for this annotation

GitHub Actions / Linting (Files Changed)

src/applications/mhv-secure-messaging/components/ComposeForm/ReplyDraftItem.jsx:137:5:React Hook useCallback has missing dependencies: 'noTimeout' and 'signOutMessage'. Either include them or remove the dependency array.
);

useEffect(
Expand All @@ -146,7 +146,7 @@
noTimeout();
};
},
[beforeUnloadHandler],

Check warning on line 149 in src/applications/mhv-secure-messaging/components/ComposeForm/ReplyDraftItem.jsx

View workflow job for this annotation

GitHub Actions / Linting (Files Changed)

src/applications/mhv-secure-messaging/components/ComposeForm/ReplyDraftItem.jsx:149:5:React Hook useEffect has a missing dependency: 'noTimeout'. Either include it or remove the dependency array.
);

const checkMessageValidity = useCallback(
Expand All @@ -157,7 +157,7 @@
messageValid = false;
}
setMessageInvalid(!messageValid);
return messageValid;
return { messageValid };
},
[messageBody],
);
Expand Down Expand Up @@ -193,18 +193,17 @@
return;
}

const isValidMessage = checkMessageValidity();
const { messageValid } = checkMessageValidity();

if (type === 'manual') {
setMessageInvalid(false);
if (isValidMessage) {
if (messageValid) {
setLastFocusableElement(e.target);
}
if (attachments.length) {
setSaveError(
ErrorMessages.ComposeForm.UNABLE_TO_SAVE_DRAFT_ATTACHMENT,
);
}
} else focusOnErrorField();
setNavigationError(
attachments.length
? {
Expand Down Expand Up @@ -234,7 +233,7 @@
body: messageBody,
};

if (isValidMessage) {
if (messageValid) {
if (!draftId) {
setFieldsString(newFieldsString);
dispatch(saveReplyDraft(replyMessage.messageId, formData, type));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
inputVaTextInput,
selectVaRadio,
selectVaSelect,
checkVaCheckbox,
} from '../../../util/testUtils';
import { drupalStaticData } from '../../fixtures/cerner-facility-mock-data.json';

Expand Down Expand Up @@ -351,10 +352,12 @@ describe('Compose form component', () => {
});

it('displays an error on attempt to save a draft with attachments', async () => {
const screen = setup(initialState, Paths.COMPOSE, {
const customProps = {
...draftMessage,
messageValid: true,
isSignatureRequired: false,
messageValid: false,
});
};
const screen = setup(initialState, Paths.COMPOSE, { draft: customProps });
const file = new File(['(⌐□_□)'], 'test.png', { type: 'image/png' });
const uploader = screen.getByTestId('attach-file-input');

Expand All @@ -371,7 +374,6 @@ describe('Compose form component', () => {
modal = screen.queryByTestId('quit-compose-double-dare');
expect(modal).to.exist;
});

expect(modal).to.have.attribute(
'modal-title',
"We can't save attachments in a draft message",
Expand Down Expand Up @@ -882,7 +884,7 @@ describe('Compose form component', () => {
expect(screen.queryByTestId('send-button')).to.not.exist;
});

it('displays alerts in Electronic Signature component if signature and checkbox is required', async () => {
it('displays alerts in Electronic Signature component if signature is required', async () => {
const screen = renderWithStoreAndRouter(
<ComposeForm recipients={initialState.sm.recipients} />,
{
Expand Down Expand Up @@ -920,17 +922,40 @@ describe('Compose form component', () => {
);
inputVaTextInput(screen.container, 'Test User', signatureTextFieldSelector);
expect(signatureTextField).to.have.attribute('error', '');
});

const checkboxSelector = `va-checkbox[label='${
it('displays an error in Electronic Signature component if checkbox is not checked', async () => {
const screen = renderWithStoreAndRouter(
<ComposeForm recipients={initialState.sm.recipients} />,
{
initialState,
reducers: reducer,
},
);
// Enters value for all other compose form fields first
const tgRecipient = initialState.sm.recipients.allowedRecipients.find(
r => r.signatureRequired,
).id;
selectVaSelect(screen.container, tgRecipient);

const checkboxSelector = `va-checkbox[label="${
ElectronicSignatureBox.CHECKBOX_LABEL
}']`;
const checkbox = screen.container.querySelector(checkboxSelector);
const sendButton = screen.getByTestId('save-draft-button');
}"]`;

fireEvent.click(sendButton);
expect(checkbox).to.have.attribute(
'error',
`${ErrorMessages.ComposeForm.CHECKBOX_REQUIRED}`,
);
await waitFor(() => {
const sendButton = screen.getByTestId('send-button');
const checkbox = screen.container.querySelector(checkboxSelector);
checkVaCheckbox(checkbox, false);

// after clicking send, validation checks on Electronic Signature component runs
fireEvent.click(sendButton);
expect(checkbox).to.have.attribute(
'error',
`${ErrorMessages.ComposeForm.CHECKBOX_REQUIRED}`,
);

checkVaCheckbox(checkbox, true);
expect(checkbox).to.have.attribute('error', '');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -57,31 +57,38 @@ describe('RecipientsSelect', () => {
const onValueChange = sinon.spy();
const setCheckboxMarked = sinon.spy();
const setElectronicSignature = sinon.spy();
const setAlertDisplayed = sinon.spy();
const customProps = {
onValueChange,
setCheckboxMarked,
setElectronicSignature,
setAlertDisplayed,
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to add validation for this spy?

};
const screen = setup({ props: customProps });
const val = recipientsList[0].id;
selectVaSelect(screen.container, val);
const select = screen.getByTestId('compose-recipient-select');

waitFor(() => {
expect(setAlertDisplayed).to.be.calledOnce;
expect(select).to.have.value(val);
expect(onValueChange.calledOnce).to.be.true;
expect(onValueChange.calledWith(recipientsList[0])).to.be.true;
});
expect(onValueChange.calledWith(recipientsList[0])).to.be.true;
});

it('displays the signature alert when a recipient with signatureRequired is selected', async () => {
const setAlertDisplayed = sinon.spy();
const customProps = {
isSignatureRequired: true,
setAlertDisplayed,
alertDisplayed: true,
};
const { getByTestId } = setup({ props: customProps });

waitFor(() => {
const alert = getByTestId('signature-alert');
expect(setAlertDisplayed).to.be.calledOnce;
expect(alert).to.exist;
expect(alert).to.contain.text(
Constants.Prompts.Compose.SIGNATURE_REQUIRED,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import PatientComposePage from '../pages/PatientComposePage';
import FolderLoadPage from '../pages/FolderLoadPage';
import { AXE_CONTEXT, Data } from '../utils/constants';

// focus validation temporary commented due to focus error (will be fixed in MHV-61146)
describe('Secure Messaging Compose Errors Keyboard Nav', () => {
beforeEach(() => {
SecureMessagingSite.login();
Expand All @@ -22,7 +21,7 @@ describe('Secure Messaging Compose Errors Keyboard Nav', () => {
PatientComposePage.getMessageBodyField().type(Data.TEST_MESSAGE_BODY);
PatientComposePage.pushSendMessageWithKeyboardPress();
PatientComposePage.verifyErrorText(Data.PLEASE_SELECT_RECIPIENT);
// PatientComposePage.verifyFocusOnErrorMessage();
PatientComposePage.verifyFocusOnErrorMessage('SELECT');

cy.injectAxe();
cy.axeCheck(AXE_CONTEXT);
Expand All @@ -36,12 +35,12 @@ describe('Secure Messaging Compose Errors Keyboard Nav', () => {
PatientComposePage.getMessageBodyField().type(Data.TEST_MESSAGE_BODY);
PatientComposePage.pushSendMessageWithKeyboardPress();
PatientComposePage.verifyErrorText(Data.PLEASE_SELECT_CATEGORY);
// PatientComposePage.verifyFocusOnErrorMessage();
PatientComposePage.verifyFocusOnErrorMessage('INPUT');

cy.injectAxe();
cy.axeCheck(AXE_CONTEXT);

// PatientComposePage.selectCategory();
PatientComposePage.selectCategory();
});

it('focus on error message for empty message subject', () => {
Expand All @@ -50,7 +49,7 @@ describe('Secure Messaging Compose Errors Keyboard Nav', () => {
PatientComposePage.getMessageBodyField().type(Data.TEST_MESSAGE_BODY);
PatientComposePage.pushSendMessageWithKeyboardPress();
PatientComposePage.verifyErrorText(Data.SUBJECT_CANNOT_BLANK);
// PatientComposePage.verifyFocusOnErrorMessage();
PatientComposePage.verifyFocusOnErrorMessage('INPUT');

cy.injectAxe();
cy.axeCheck(AXE_CONTEXT);
Expand All @@ -66,7 +65,7 @@ describe('Secure Messaging Compose Errors Keyboard Nav', () => {
});
PatientComposePage.pushSendMessageWithKeyboardPress();
PatientComposePage.verifyErrorText(Data.BODY_CANNOT_BLANK);
// PatientComposePage.verifyFocusOnErrorMessage();
PatientComposePage.verifyFocusOnErrorMessage('TEXTAREA');

cy.injectAxe();
cy.axeCheck(AXE_CONTEXT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import SecureMessagingSite from '../sm_site/SecureMessagingSite';
import PatientInboxPage from '../pages/PatientInboxPage';
import PatientComposePage from '../pages/PatientComposePage';
import requestBody from '../fixtures/message-compose-request-body.json';
import { AXE_CONTEXT } from '../utils/constants';
import { AXE_CONTEXT, Locators } from '../utils/constants';

describe('Check confirmation message after save draft', () => {
it('Check confirmation message after save draft', () => {
Expand All @@ -22,9 +22,12 @@ describe('Check confirmation message after save draft', () => {
cy.injectAxe();
cy.axeCheck(AXE_CONTEXT);

PatientComposePage.verifyDraftSaveButtonOnFocus();
PatientComposePage.verifyAlertFocusFocus();
cy.get(Locators.BUTTONS.ALERT_CLOSE).click();

cy.get('.sm-breadcrumb-list-item')
.find('a')
.click();
cy.get(Locators.BACK_TO).click();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ class ContactListPage {
.find(`a[href*="contact"]`)
.click({ force: true });

cy.contains(`Delete draft`).click({ force: true });
cy.url().should(`include`, `${Paths.UI_MAIN}/contact-list`);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ class PatientComposePage {
};

selectRecipient = (index = 1) => {
cy.get(Locators.ALERTS.REPT_SELECT).click();
cy.get(Locators.ALERTS.REPT_SELECT)
.shadow()
.find('select')
Expand Down Expand Up @@ -426,8 +425,8 @@ class PatientComposePage {
.should('be.visible');
};

verifyDraftSaveButtonOnFocus = () => {
cy.get(Locators.BUTTONS.SAVE_DRAFT)
verifyAlertFocusFocus = () => {
cy.get(`.first-focusable-child`)
.should('exist')
.and('be.focused');
};
Expand Down Expand Up @@ -503,7 +502,6 @@ class PatientComposePage {

backToInbox = () => {
cy.get(Locators.BACK_TO).click();
cy.get('[visible=""] > [secondary=""]').click({ force: true });
};
}

Expand Down
Loading
Loading