LG-8407: PO Search: search button shows loading state

[allthesignals]

🎫 Ticket

Link to the relevant ticket.

🛠 Summary of changes

I used <SpinnerButton/> in place of <Button/> in order to show loading state to the user.

I also swapped in usage of useSWR for more declarative component style.

Note: the button only shows loading when the app is looking up an address. It does not continue to show loading when the second trip is made to query for PO locations.

^ Open to suggestions on how to organize things so that loading continues into the second request... one way is to provide the toggleSpinner in the onAddressFound callback, and call it when appropriate from the consuming end...

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

• Manual check in develop

👀 Screenshots

If relevant, include a screenshot or screen capture of the changes.

Before:

After:

[aduth]

LGTM 👍

It'd be nice to have some spec coverage for these components so that we can be more confident in these refactors.

[allthesignals]

This makes me reconsider how opinionated this component should be about the return value... I don't think it's appropriate for it only return the best match, and maybe that should be dealt with on the consumer side. This component should return all results.

[allthesignals]

Failure likely due to leakiness in stubbing...

  AssertionError: expected fetch to have been called with arguments '/verify/in_person/usps_locations', {
  test: [Function (anonymous)],
  message: 'match(body: {"address":{"street_address":"100 Main St","city":"South Fulton","state":"TN","zip_code":"38257","address":"100 Main St, South Fulton, Tennessee, 38257"}}, method: post)'
}
'"/verify/in_person/usps_locations"'
{
  method: 'post',
  headers: Headers {
    map: { 'content-type': 'application/json', accept: 'application/json' }
  },
  body: '{"address":{"street_address":"100 Main St E","city":"Bronwood","state":"GA","zip_code":"39826","address":"100 Main St E, Bronwood, Georgia, 39826"}}'
} match(body: {"address":{"street_address":"100 Main St","city":"South Fulton","state":"TN","zip_code":"38257","address":"100 Main St, South Fulton, Tennessee, 38257"}}, method: post)

[aduth]

Dunno if you deleted a comment, but I arrived here from an email notification about this type assignment. I think it might be better to defer the type to how it's defined by @18f/identity-form-steps:

import type { RegisterFieldCallback } from '@18f/identity-form-steps';

interface AddressSearchProps {
  // ...
  registerField?: RegisterFieldCallback;
}

[aduth]

(It was your comment at #7502 (comment), maybe the GitHub UI was being slow to update)

[allthesignals]

Yeah I was interested in the difference between void and undefined, and thought they were interchangeable in TS but was wrong!

This change was made so that i could test without need to deal with the validated field provider stuff.

[allthesignals]

I think in order to use MSW, we'll need to rewrite some of these form-steps-wait-spec. These use a mix of stub/mock, and I'm not sure it'll make much of a difference.

[allthesignals]

Sloppy workaround pushed

[allthesignals]

Real fetch is needed for this, but changing that in the spec_helper breaks other tests

[aduth]

The useDefineProperty helper may have helped for the approach we ended up taking, as an alternative to duplicating the restored original fetch. Or something like...

let originalFetch;

before(() => {
  originalFetch = window.fetch;
  window.fetch = fetch;
});

after(() => {
  window.fetch = originalFetch;
});

But hopefully we can get native fetch working so we won't need this anyways.