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

Fixed scroll/focus logic on close of modal #28266

Merged
merged 13 commits into from Mar 4, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -63,7 +63,7 @@ export const submitRepresentativeReport = newReport => {
} catch (error) {
Sentry.withScope(scope => {
scope.setExtra('error', error);
Sentry.captureMessage('Error fetching accredited representatives');
Sentry.captureMessage('Error submitting representative report');
cosu419 marked this conversation as resolved.
Show resolved Hide resolved
});

dispatch({
Expand Down
@@ -1,4 +1,9 @@
import { fetchAndUpdateSessionExpiration as fetch } from '@department-of-veterans-affairs/platform-utilities/api';
/* eslint-disable camelcase */

cosu419 marked this conversation as resolved.
Show resolved Hide resolved
import {
fetchAndUpdateSessionExpiration as fetch,
apiRequest,
} from '@department-of-veterans-affairs/platform-utilities/api';
import { getApi, resolveParamsWithUrl, endpointOptions } from '../config';

class RepresentativeFinderApi {
Expand Down Expand Up @@ -41,6 +46,9 @@ class RepresentativeFinderApi {
if (!response.ok) {
throw Error(response.statusText);
}
const csrf = response.headers.get('X-CSRF-Token');
localStorage.setItem('csrfToken', csrf);

return response.json();
})
.then(res => {
Expand All @@ -57,30 +65,16 @@ class RepresentativeFinderApi {
}

static reportResult(newReport) {
const reportRequestBody = {
representativeId: newReport.representativeId,
flags: [],
};

const startTime = new Date().getTime();

for (const [flagType, flaggedValue] of Object.entries(newReport.reports)) {
if (flaggedValue !== null) {
reportRequestBody.flags.push({
flagType,
flaggedValue,
});
}
}

const { requestUrl, apiSettings } = getApi(
endpointOptions.flagReps,
'POST',
reportRequestBody,
newReport,
);

return new Promise((resolve, reject) => {
fetch(requestUrl, apiSettings)
apiRequest(requestUrl, apiSettings)
.then(response => {
if (!response.ok) {
throw Error(response.statusText);
Expand Down
@@ -1,10 +1,11 @@
/* eslint-disable camelcase */
cosu419 marked this conversation as resolved.
Show resolved Hide resolved

import React, { useState } from 'react';
import PropTypes from 'prop-types';
import {
VaModal,
VaCheckboxGroup,
} from '@department-of-veterans-affairs/component-library/dist/react-bindings';
import { snakeCase } from 'lodash';

const ReportModal = ({
representativeName,
Expand All @@ -15,8 +16,8 @@
existingReports,
onCloseModal,
submitRepresentativeReport,
handleOtherInputChangeTestId,

Check warning on line 19 in src/applications/representative-search/components/results/ReportModal.jsx

View workflow job for this annotation

GitHub Actions / Linting (Files Changed)

src/applications/representative-search/components/results/ReportModal.jsx:19:3:'handleOtherInputChangeTestId' is missing in props validation
testReportObject,

Check warning on line 20 in src/applications/representative-search/components/results/ReportModal.jsx

View workflow job for this annotation

GitHub Actions / Linting (Files Changed)

src/applications/representative-search/components/results/ReportModal.jsx:20:3:'testReportObject' is missing in props validation
}) => {
const [reportObject, setReportObject] = useState({
phone: null,
Expand Down Expand Up @@ -78,17 +79,15 @@
};

const onSubmitModal = () => {
const formattedReportObject = { representativeId, reports: {} };
const formattedReportObject = {
representativeId,
reports: {},
};

// push non-null items to reports object
Object.keys(reportObject).forEach(prop => {
if (reportObject[prop] !== null) {
if (prop === 'phone') {
formattedReportObject.reports[snakeCase('phoneNumber')] =
reportObject.phone;
} else {
formattedReportObject.reports[prop] = reportObject[prop];
}
formattedReportObject.reports[prop] = reportObject[prop];
}
});

Expand Down Expand Up @@ -126,7 +125,7 @@
{/* These buttons trigger methods for unit testing - temporary workaround for shadow root issues with va checkboxes */}
{handleOtherInputChangeTestId ? (
<>
<button

Check warning on line 128 in src/applications/representative-search/components/results/ReportModal.jsx

View workflow job for this annotation

GitHub Actions / Linting (Files Changed)

src/applications/representative-search/components/results/ReportModal.jsx:128:13:A control must be associated with a text label.

Check warning on line 128 in src/applications/representative-search/components/results/ReportModal.jsx

View workflow job for this annotation

GitHub Actions / Linting (Files Changed)

src/applications/representative-search/components/results/ReportModal.jsx:128:13:The <va-button> Web Component should be used instead of the button HTML element.
id="handle-checkbox-change-test-button"
type="button"
onClick={() =>
Expand All @@ -135,7 +134,7 @@
})
}
/>
<button

Check warning on line 137 in src/applications/representative-search/components/results/ReportModal.jsx

View workflow job for this annotation

GitHub Actions / Linting (Files Changed)

src/applications/representative-search/components/results/ReportModal.jsx:137:13:A control must be associated with a text label.

Check warning on line 137 in src/applications/representative-search/components/results/ReportModal.jsx

View workflow job for this annotation

GitHub Actions / Linting (Files Changed)

src/applications/representative-search/components/results/ReportModal.jsx:137:13:The <va-button> Web Component should be used instead of the button HTML element.
id="handle-other-input-change-test-button"
type="button"
onClick={() =>
Expand All @@ -151,12 +150,12 @@
) : null}
{testReportObject ? (
<>
<button

Check warning on line 153 in src/applications/representative-search/components/results/ReportModal.jsx

View workflow job for this annotation

GitHub Actions / Linting (Files Changed)

src/applications/representative-search/components/results/ReportModal.jsx:153:13:A control must be associated with a text label.

Check warning on line 153 in src/applications/representative-search/components/results/ReportModal.jsx

View workflow job for this annotation

GitHub Actions / Linting (Files Changed)

src/applications/representative-search/components/results/ReportModal.jsx:153:13:The <va-button> Web Component should be used instead of the button HTML element.
id="set-report-object-button"
type="button"
onClick={() => setReportObject({ ...testReportObject })}
/>
<button

Check warning on line 158 in src/applications/representative-search/components/results/ReportModal.jsx

View workflow job for this annotation

GitHub Actions / Linting (Files Changed)

src/applications/representative-search/components/results/ReportModal.jsx:158:13:A control must be associated with a text label.

Check warning on line 158 in src/applications/representative-search/components/results/ReportModal.jsx

View workflow job for this annotation

GitHub Actions / Linting (Files Changed)

src/applications/representative-search/components/results/ReportModal.jsx:158:13:The <va-button> Web Component should be used instead of the button HTML element.
id="submit-modal-test-button"
type="button"
onClick={() => onSubmitModal()}
Expand Down
@@ -1,6 +1,9 @@
import React, { useState } from 'react';
import React, { useState, useEffect, useRef } from 'react';
import PropTypes from 'prop-types';
import { scrollTo } from 'platform/utilities/ui';
import {
focusElement,
scrollTo,
} from '@department-of-veterans-affairs/platform-utilities/ui';
import ReportModal from './ReportModal';
import { parsePhoneNumber } from '../../utils/phoneNumbers';

Expand All @@ -24,9 +27,9 @@ const SearchResult = ({
}) => {
const [reportModalIsShowing, setReportModalIsShowing] = useState(false);

const { contact, extension } = parsePhoneNumber(phone);
const prevReportCount = useRef(reports?.length || 0);

const scrollElementId = `result-${representativeId}`;
const { contact, extension } = parsePhoneNumber(phone);

const addressExists = addressLine1 || city || stateCode || zipCode;

Expand All @@ -45,9 +48,20 @@ const SearchResult = ({

const closeReportModal = () => {
setReportModalIsShowing(false);
scrollTo(scrollElementId);
scrollTo(`#report-button-${representativeId}`);
focusElement(`#report-button-${representativeId}`);
};

useEffect(
() => {
if (reports?.length > prevReportCount) {
scrollTo(`#thank-you-alert-${representativeId}`);
focusElement(`#thank-you-alert-${representativeId}`);
}
},
[reports],
);

return (
<div className="report-outdated-information-modal">
{/* Trigger methods for unit testing - temporary workaround for shadow root issues */}
Expand Down Expand Up @@ -151,16 +165,20 @@ const SearchResult = ({
{reports && (
<div className="report-thank-you-alert">
<va-alert
class="vads-u-margin-bottom--2"
class="thank-you-alert vads-u-margin-bottom--2"
close-btn-aria-label="Close notification"
disable-analytics="false"
tabIndex={-1}
full-width="false"
slim
status="info"
uswds
visible="true"
>
<p className="vads-u-margin-y--0">
<p
id={`thank-you-alert-${representativeId}`}
className="vads-u-margin-y--0"
>
Thanks for reporting outdated information.
</p>
</va-alert>
Expand All @@ -171,6 +189,8 @@ const SearchResult = ({
onClick={() => {
setReportModalIsShowing(true);
}}
tabIndex={-1}
id={`report-button-${representativeId}`}
secondary
text="Report outdated information"
uswds
Expand Down
27 changes: 26 additions & 1 deletion src/applications/representative-search/config.js
Expand Up @@ -39,6 +39,25 @@ const baseUrl =
? `https://staging-api.va.gov`
: `${environment.API_URL}`;

export const formatReportBody = newReport => {
const reportRequestBody = {
representative_id: newReport.representativeId,
flags: [],
};

for (const [flag_type, flagged_value] of Object.entries(newReport.reports)) {
if (flagged_value !== null) {
reportRequestBody.flags.push({
// convert 'phone' to snakecase 'phone_number' before pushing
flag_type: flag_type === 'phone' ? 'phone_number' : flag_type,
flagged_value,
});
}
}

return reportRequestBody;
};

/**
* Build requestUrl and settings for api calls
* * @param endpoint {String} eg '/vso_accredited_representatives'
Expand All @@ -51,6 +70,12 @@ export const getApi = (endpoint, method = 'GET', requestBody) => {

const csrfToken = localStorage.getItem('csrfToken');

let formattedReportBody;

if (method === 'POST') {
formattedReportBody = formatReportBody(requestBody);
}

const apiSettings = {
mode: 'cors',
method,
Expand All @@ -66,7 +91,7 @@ export const getApi = (endpoint, method = 'GET', requestBody) => {
// undefined for all requests that use this config.
'Source-App-Name': manifest.entryName,
},
body: JSON.stringify(requestBody) || null,
body: JSON.stringify(formattedReportBody) || null,
};

return { requestUrl, apiSettings };
Expand Down
4 changes: 1 addition & 3 deletions src/applications/representative-search/containers/App.jsx
Expand Up @@ -16,9 +16,7 @@ function App({ children }) {
TOGGLE_NAMES,
} = useFeatureToggle();

const appEnabled = useToggleValue(
TOGGLE_NAMES.findARepresentativeEnableFrontend,
);
const appEnabled = useToggleValue(TOGGLE_NAMES.findARepresentativeEnabled);

const togglesLoading = useToggleLoadingValue();

Expand Down
@@ -1,13 +1,13 @@
const generateFeatureToggles = (toggles = {}) => {
const { findARepresentativeEnableFrontend = false } = toggles;
const { findARepresentativeEnabled = false } = toggles;

return {
data: {
type: 'feature_toggles',
features: [
{
name: 'find_a_representative_enable_frontend',
value: findARepresentativeEnableFrontend,
name: 'find_a_representative',
value: findARepresentativeEnabled,
},
],
},
Expand Down
@@ -1,6 +1,11 @@
import { expect } from 'chai';
import environment from '@department-of-veterans-affairs/platform-utilities/environment';
import { resolveParamsWithUrl, getApi, endpointOptions } from '../config';
import {
resolveParamsWithUrl,
getApi,
endpointOptions,
formatReportBody,
} from '../config';

describe('Locator url and parameters builder', () => {
const address = '43210';
Expand Down Expand Up @@ -90,6 +95,24 @@ describe('Locator url and parameters builder', () => {
expect(apiSettings?.headers?.['X-CSRF-Token']).to.eql('12345');
});

it('should format report object into snake case POST request body', () => {
const reportObject = {
representativeId: 123,
reports: {
phone: '644-465-8493',
email: 'example@rep.com',
address: '123 Any Street',
other: 'other comment',
},
};

const formattedReportBody = JSON.stringify(formatReportBody(reportObject));

expect(formattedReportBody).to.eql(
'{"representative_id":123,"flags":[{"flag_type":"phone_number","flagged_value":"644-465-8493"},{"flag_type":"email","flagged_value":"example@rep.com"},{"flag_type":"address","flagged_value":"123 Any Street"},{"flag_type":"other","flagged_value":"other comment"}]}',
);
});

it('should exclude null params from request', () => {
const { requestUrl } = getApi(endpointOptions.fetchOtherReps);

Expand Down
Expand Up @@ -7,9 +7,7 @@ describe('Accessibility', () => {
cy.viewport(1200, 700);
cy.intercept('GET', '/v0/feature_toggles*', {
data: {
features: [
{ name: 'find_a_representative_enable_frontend', value: true },
],
features: [{ name: 'find_a_representative_enabled', value: true }],
},
});
cy.intercept('GET', '/v0/maintenance_windows', []);
Expand Down
Expand Up @@ -20,9 +20,7 @@ describe('User geolocation', () => {
cy.intercept('GET', '/geocoding/**/*', mockLaLocation).as('caLocation');
cy.intercept('GET', '/v0/feature_toggles*', {
data: {
features: [
{ name: 'find_a_representative_enable_frontend', value: true },
],
features: [{ name: 'find_a_representative_enabled', value: true }],
},
});
cy.visit('/get-help-from-accredited-representative/find-rep/');
Expand Down
Expand Up @@ -17,9 +17,7 @@ describe('Mobile', () => {
beforeEach(() => {
cy.intercept('GET', '/v0/feature_toggles*', {
data: {
features: [
{ name: 'find_a_representative_enable_frontend', value: true },
],
features: [{ name: 'find_a_representative_enabled', value: true }],
},
});
cy.intercept('GET', '/v0/maintenance_windows', []);
Expand Down
Expand Up @@ -30,9 +30,7 @@ describe('Representative Search', () => {
beforeEach(() => {
cy.intercept('GET', '/v0/feature_toggles*', {
data: {
features: [
{ name: 'find_a_representative_enable_frontend', value: true },
],
features: [{ name: 'find_a_representative_enabled', value: true }],
},
});
cy.intercept('GET', '/v0/maintenance_windows', []);
Expand Down
Expand Up @@ -4,9 +4,7 @@ describe('Find a Representative error handling', () => {
beforeEach(() => {
cy.intercept('GET', '/v0/feature_toggles*', {
data: {
features: [
{ name: 'find_a_representative_enable_frontend', value: true },
],
features: [{ name: 'find_a_representative_enabled', value: true }],
},
});
cy.intercept('GET', '/v0/maintenance_windows', []);
Expand Down
Expand Up @@ -98,6 +98,7 @@ export default Object.freeze({
financialStatusReportReviewPageNavigation:
'financial_status_report_review_page_navigation',
findARepresentativeEnableFrontend: 'find_a_representative_enable_frontend',
findARepresentativeEnabled: 'find_a_representative_enabled',
form2010206: 'form2010206',
form2110210: 'form2110210',
form210845: 'form210845',
Expand Down