Skip to content

LG-8330: Allow user to select USPS location when search box is empty #7760

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.

Sign up for GitHub

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

Merged
eileen-nava merged 5 commits into from
Feb 3, 2023
Merged

LG-8330: Allow user to select USPS location when search box is empty #7760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3820b29
user can't proceed past empty textbox, but no validation error displa…
eileen-nava Feb 1, 2023
946b08b
disable search input field when user selects a location
eileen-nava Feb 2, 2023
e659cbb
clean up unneeded changes
eileen-nava Feb 2, 2023
7a3f978
changelog: Bug Fixes, In-person proofing, allow user to select USPS l…
eileen-nava Feb 2, 2023
925989f
test that user can select location when search box is empty
eileen-nava Feb 3, 2023
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
3 changes: 3 additions & 0 deletions app/javascript/packages/document-capture/components/address-search.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ interface AddressSearchProps {
onFoundLocations?: (locations: FormattedLocation[] | null | undefined) => void;
onLoadingLocations?: (isLoading: boolean) => void;
onError?: (error: Error | null) => void;
disabled?: boolean;
}

function AddressSearch({
Expand All @@ -169,6 +170,7 @@ function AddressSearch({
onFoundLocations = () => undefined,
onLoadingLocations = () => undefined,
onError = () => undefined,
disabled = false,
}: AddressSearchProps) {
const { t } = useI18n();
const spinnerButtonRef = useRef<SpinnerButtonRefHandle>(null);
Expand Down Expand Up @@ -227,6 +229,7 @@ function AddressSearch({
onChange={onTextInputChange}
label={t('in_person_proofing.body.location.po_search.address_search_label')}
hint={t('in_person_proofing.body.location.po_search.address_search_hint')}
disabled={disabled}
Copy link
Copy Markdown
Contributor

@allthesignals allthesignals Feb 2, 2023

Choose a reason for hiding this comment

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

Interesting... how does this fix the issue? Does the FormSteps stuff ignore disabled fields even after invalidation?

Copy link
Copy Markdown
Contributor

@allthesignals allthesignals Feb 2, 2023

Choose a reason for hiding this comment

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

@NavaTim I'm just curious — how does this work/fix the bug?

Copy link
Copy Markdown
Contributor

@JackRyan1989 JackRyan1989 Feb 2, 2023

Choose a reason for hiding this comment

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

@allthesignals: I think this answers what you're asking, but please say so if I missed your q. @eileen-nava filled me in on the inner workings here. The form steps component is just a standard form element, so when you pass an attribute of 'disabled' to that form field, it does not validate that field. TIL: MDN Disabled Overview

Copy link
Copy Markdown
Contributor

@allthesignals allthesignals Feb 2, 2023

Choose a reason for hiding this comment

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

Oh, great! I assumed FormSteps was doing validation... sounds like it's being done natively!

Copy link
Copy Markdown
Contributor

@NavaTim NavaTim Feb 2, 2023

Choose a reason for hiding this comment

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

@allthesignals The ValidatedFieldElement hooks into the invalid event, which doesn't trigger when the element is disabled.

Relevant spec sections:

valid = element.checkValidity()
Returns true if the element's value has no validity problems; false otherwise. Fires an invalid event at the element in the latter case.

Source

Note that the invalid event in the above source is mentioned as being triggered by the constraint validation API.

Constraint validation: If an element is disabled, it is barred from constraint validation.

Source

A submittable element is a candidate for constraint validation except when a condition has barred the element from constraint validation. (For example, an element is barred from constraint validation if it is an object element.)

Source

Copy link
Copy Markdown
Contributor

@allthesignals allthesignals Feb 2, 2023

Choose a reason for hiding this comment

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

Fascinating. Thank you Tim!

/>
</ValidatedField>
<div className="margin-y-5">
Expand Down
34 changes: 34 additions & 0 deletions .../packages/document-capture/components/in-person-location-post-office-search-step.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,4 +368,38 @@ describe('InPersonLocationStep', () => {
expect(moreResults).to.be.empty();
});
});

context('user deletes text from searchbox after location results load', () => {
beforeEach(() => {
server.use(
rest.post(ADDRESS_SEARCH_URL, (_req, res, ctx) =>
res(ctx.json(DEFAULT_RESPONSE), ctx.status(200)),
),
rest.post(LOCATIONS_URL, (_req, res, ctx) => res(ctx.json([{ name: 'Baltimore' }]))),
);
});

it('allows user to select a location', async () => {
const { findAllByText, findByLabelText, findByText, queryByText } = render(
<InPersonLocationPostOfficeSearchStep {...DEFAULT_PROPS} />,
{ wrapper },
);
await userEvent.type(
await findByLabelText('in_person_proofing.body.location.po_search.address_search_label'),
'Evergreen Terrace Springfield',
);

await userEvent.click(
await findByText('in_person_proofing.body.location.po_search.search_button'),
);

await userEvent.clear(
await findByLabelText('in_person_proofing.body.location.po_search.address_search_label'),
);
Comment on lines +396 to +398
Copy link
Copy Markdown
Contributor

@NavaTim NavaTim Feb 3, 2023

Choose a reason for hiding this comment

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

Probably would move this and related prereq steps to the beforeEach, but it's not very important at the moment because we only have one it in this context.


await userEvent.click(findAllByText('in_person_proofing.body.location.location_button')[0]);

expect(await queryByText('in_person_proofing.body.location.inline_error')).to.be.null();
});
});
});
39 changes: 22 additions & 17 deletions ...cript/packages/document-capture/components/in-person-location-post-office-search-step.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,16 @@ import InPersonLocations, { FormattedLocation } from './in-person-locations';

function InPersonLocationPostOfficeSearchStep({ onChange, toPreviousStep, registerField }) {
const { t } = useI18n();
const [inProgress, setInProgress] = useState(false);
const [isLoadingLocations, setLoadingLocations] = useState(false);
const [autoSubmit, setAutoSubmit] = useState(false);
const [inProgress, setInProgress] = useState<boolean>(false);
const [isLoadingLocations, setLoadingLocations] = useState<boolean>(false);
const [autoSubmit, setAutoSubmit] = useState<boolean>(false);
const { setSubmitEventMetadata } = useContext(AnalyticsContext);
const [locationResults, setLocationResults] = useState<FormattedLocation[] | null | undefined>(
null,
);
const [foundAddress, setFoundAddress] = useState<LocationQuery | null>(null);
const [apiError, setApiError] = useState<Error | null>(null);
const [disabledAddressSearch, setDisabledAddressSearch] = useState<boolean>(false);

// ref allows us to avoid a memory leak
const mountedRef = useRef(false);
Expand All @@ -43,6 +44,12 @@ function InPersonLocationPostOfficeSearchStep({ onChange, toPreviousStep, regist
setSubmitEventMetadata({ selected_location: selectedLocationAddress });
onChange({ selectedLocationAddress });
if (autoSubmit) {
setDisabledAddressSearch(true);
setTimeout(() => {
if (mountedRef.current) {
setDisabledAddressSearch(false);
}
}, 250);
return;
}
// prevent navigation from continuing
Expand All @@ -52,28 +59,25 @@ function InPersonLocationPostOfficeSearchStep({ onChange, toPreviousStep, regist
}
const selected = transformKeys(selectedLocation, snakeCase);
setInProgress(true);
await request(LOCATIONS_URL, {
json: selected,
method: 'PUT',
})
.then(() => {
if (!mountedRef.current) {
return;
}
try {
await request(LOCATIONS_URL, {
json: selected,
method: 'PUT',
});
if (mountedRef.current) {
setAutoSubmit(true);
setImmediate(() => {
// continue with navigation
e.target.click();
// allow process to be re-triggered in case submission did not work as expected
setAutoSubmit(false);
});
})
.finally(() => {
if (!mountedRef.current) {
return;
}
}
} finally {
if (mountedRef.current) {
setInProgress(false);
});
}
}
},
[locationResults, inProgress],
);
Expand All @@ -93,6 +97,7 @@ function InPersonLocationPostOfficeSearchStep({ onChange, toPreviousStep, regist
onFoundLocations={setLocationResults}
onLoadingLocations={setLoadingLocations}
onError={setApiError}
disabled={disabledAddressSearch}
/>
{locationResults && foundAddress && !isLoadingLocations && (
<InPersonLocations
Expand Down