Related Persons modal should provide better error feedback

[jackcmeyer]

🚀 Feature Proposal

I would like the add related persons modal to provide better feedback for error messages.

Currently, the only error message is a banner error message.

After the completion of this issue, I would like it to provide feedback at the field level when possible.

The following should happen:

• When a Related Persons is not filled in, the related person should highlight in red, and provide feedback saying Related Person is required.
• When a Relationship Type is not filled in, the relationship type field should highlight in red, and provide feedback saying Relationship Type is required.
• The banner message should read Unable to add new related person.

Motivation

Better user experience.

[bmoore235]

I would love to tackle this issue, along with New Related Persons Modal should mark required fields #1927

[jackcmeyer]

@bmoore235 sounds good! I've assigned this to you.

[bmoore235]

Sorry this has taken so long. These first couple weeks of isolation made me less productive than I anticipated. I'll have the pull request on this in today.

[jamesinsf-git]

@bmoore235 I started looking at this recently and got stuck on this issue with Typeahead:
ericgio/react-bootstrap-typeahead#432

Basically, the Feedback component does not play nicely with Typeahead. Not sure if you've figured out a workaround.

update: nevermind, I see you're not using feedback for the Typeahead input. gotcha.

[bmoore235]

Ya typeahead doesn't support feedback unfortunately.

I have the feature set, but I'm having some issues with testing. I have the following code, but neither test condition triggers the patient.relatedPersons.error.relatedPersonRequired text.

it('should provide feedback when no relationship given', () => {
      act(() => {
        const patientTypeahead = wrapper.find(Typeahead)
        patientTypeahead.prop('onChange')([{ id: '123' }])
      })
      wrapper.update()

      act(() => {
        ;(wrapper.find(Modal).prop('successButton') as any).onClick(
          {} as React.MouseEvent<HTMLButtonElement, MouseEvent>,
        )
      })
      ....
      ....
      ....
    })

    it('should provide feedback when no related person given', () => {
      act(() => {
        const relationshipTypeTextInput = wrapper.findWhere((w) => w.prop('name') === 'type')
        relationshipTypeTextInput.prop('onChange')({ target: { value: 'relationship' } })
      })
      wrapper.update()

      act(() => {
        ;(wrapper.find(Modal).prop('successButton') as any).onClick(
          {} as React.MouseEvent<HTMLButtonElement, MouseEvent>,
        )
      })
   ....
   ....
   ....
    })

[jamesinsf-git]

Is it only a test issue? Does it work when you test this manually?

[jackcmeyer]

@bmoore235 I think this issue is that the onChange function for the Typeahead component is async.

So you'll need to make the tests async, the act async, and then await the onChange.

Something like

it('should provide feedback when no relationship given', async () => {
      await act(async () => {
        const patientTypeahead = wrapper.find(Typeahead)
        await patientTypeahead.prop('onChange')([{ id: '123' }])
      })
      wrapper.update()

      act(() => {
        ;(wrapper.find(Modal).prop('successButton') as any).onClick(
          {} as React.MouseEvent<HTMLButtonElement, MouseEvent>,
        )
      })
      ....
      ....
      ....
    })

    it('should provide feedback when no related person given', async () => {
      await act((async ) => {
        const relationshipTypeTextInput = wrapper.findWhere((w) => w.prop('name') === 'type')
        await relationshipTypeTextInput.prop('onChange')({ target: { value: 'relationship' } })
      })
      wrapper.update()

      act(() => {
        ;(wrapper.find(Modal).prop('successButton') as any).onClick(
          {} as React.MouseEvent<HTMLButtonElement, MouseEvent>,
        )
      })
   ....
   ....
   ....
    })

[jackcmeyer]

@bmoore235 feel free to open a pull request with the code you have (preferably compiling code, however tests don't have to pass) and we can provide help since we will be able to see all of the changes!