Bad handling of focus on ErrorSummary component

[oddjones]

Hi Andy - sorry to fill your issues thread mate, but I've come across a more general issue with the Formik example (which shows I'm persevering though eh?) - there's a validation snafu which I think is related to the issue identified in this thread on Formik's site ( https://github.com/formium/formik/issues/882 ) - The form validates fine but there's an issue when you come to fix the errors it points out - as soon as you type a character in one of the failing fields the field loses the focus, stopping you from typing until you click back in the field. The thread I've highlighted (which is primarily concerned with styled components) suggests creating the component outside of the Formik component then referencing it - I've tried this with no success... Wondering where to go next.

[andymantell]

oh yes I see the bug. Let me see if I can fix it in the examples...

Sorry I haven't got back to you about #99 yet, I'm struggling to find time at the moment

[oddjones]

No worries dude - I figured this one might be a bit more important as it's a more general fail - and failing is learning, right ;-) ?

[andymantell]

I've not been able to fix it in my lunch break, I will see what I can do this evening.

[oddjones]

No worries mate - I'm trying stuff my end - If I get anywhere I'll let you know ;-)

[andymantell]

Yeah let me know what you find. The "worry" is whether it's not in my formik example code but inside the govuk-react-jsx component itself. My head has been out of the React space for a few months now so having to get back up to speed!

[oddjones]

I feel it's more to do with the ErrorSummary component than anything else - without it on the page everything works as expected - I notice there's a useEffect on line 29 of this component that appears to set the focus to the "errorSummaryRef" - When I removed the "errorList" from the dependency array (ensuring it only runs the useEffect once) - that seems to have solved the problem? - suggesting maybe the ErrorSummary was hijacking the focus every time a field was being blurred?

[andymantell]

oh god, yes, you're right. That focuses itself when error messages are displayed, because that's the behaviour from govuk-frontend - based on validation only occurring when the submit button is pressed. There's quite a lot of accessibility pitfalls with dynamic validation like this which is I think why they don't recommend it.

Seems to me that there's a few of actions needed out of this:

1) Provide the ability to disable the automatic focusing behaviour via a prop on the ErrorSummary component. Even if the recommendation is not to do this kind of validation, people will use it so should probably be supported.

2) Either remove the dynamic validation example, or make a note that it may have accessibility issues.

3) Unrelated - but it looks like the react-hook-form examples are broken too :-|

[oddjones]

I agree with GDS to be fair on the whole dynamic validation stuff - it's nice to offer some client side stuff as progressive enhancement but it shouldn't be in the driving seat IMHO - bizarrely I'm trying to make this work with SSR on a NextJS implementation - I'm hoping to make use of the same yup schema to drive validation against the api ideally - this is just my direction of travel ;-)

You can't get everything right 1st time dude - this is an amazing library and well worth the time you've clearly invested in it - hope you don't mind my pecking your head with issues as I stumble across 'em

[andymantell]

no of course, these issues are all extremely welcome. I won't pretend I enjoy receiving issues! :-) But that's just how open source works innit.

I think this one isn't as thorny as it first appeared thankfully - if I do the things set out above then I think I can make it work.

In the short term I suggest you copy and paste this code...

https://github.com/surevine/govuk-react-jsx/blob/main/src/govuk/components/error-summary/index.js

...into a component local to your project and simply remove the useEffect which is calling out to the govuk javascript. That will remove the automatic focusing behaviour for now.

[andymantell]

@oddjones Just to say I haven't forgotten about this. I am going to find some time this weekend to take a look. I have a plan I think will work - basically I'm going to remove the automatic focusing from this component completely. Then when you use it, you will be able to pass a ref into the component which will give you a handle to the actual error summary dom element. Then it will be up to you to decide when to focus the error summary. So the error summary will update dynamically, but you only focus it to draw a screen reader's attention to it when some significant action has happened (Most likely only when you click the actual submit button). Hope that sounds ok.

[andymantell]

@oddjones I've made a fix and it seems to be working. I'm going to revisit this tomorrow morning and read what I've done again with fresh eyes before I ship it.

[andymantell]

Todo:

• [x] Linting
• [ ] React-hook-form equivalent

[oddjones]

Cool - I'll pull it when I get the chance this week - i've been put onto another project for now so don't rush on my part - does this fix the other issue as well? (the button issue https://github.com/surevine/govuk-react-jsx/issues/99 )

[andymantell]

If you're pulling this repo in directly you'll need to build it - you can just pull it in from github. When I'm working locally I have a little script to do this:

npm run build
mkdir -p ../govuk-react-jsx-examples/node_modules/govuk-react-jsx
cp -r dist/* ../govuk-react-jsx-examples/node_modules/govuk-react-jsx

(Replace govuk-react-jsx-examples with your folder name obvs)

And then you would need to make the changes from govuk-react-jsx-examples in your repo.

I'll publish it in the next few days though once I'm confident it does what we need so don't worry too much.

[andymantell]

oh and no, it doesn't fix the button issue

[andymantell]

@oddjones I've made it a bit better and that's now released as 4.1.1 (It shouldn't be focusing the error summary too much now). The demo code at https://github.com/surevine/govuk-react-jsx-examples/pull/1 doesn't quite work though - I think because it's getting a new ref every time it re-renders the error summary. I only noticed this after I released it :-(

4.1.1 is better than 4.1.0 so I'm going to leave it there. But it's not completed fixed and finished. I will try again...

[oddjones]

Very good mate

On Sat, 30 Jan 2021 at 12:30, Andy Mantell notifications@github.com wrote:

@oddjones https://github.com/oddjones I've made it a bit better and that's now released as 4.1.1 (It shouldn't be focusing the error summary too much now). The demo code at surevine/govuk-react-jsx-examples#1 https://github.com/surevine/govuk-react-jsx-examples/pull/1 doesn't quite work though - I think because it's getting a new ref every time it re-renders the error summary. I only noticed this after I released it :-(

4.1.1 is better than 4.1.0 so I'm going to leave it there. But it's not completed fixed and finished. I will try again...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/surevine/govuk-react-jsx/issues/100#issuecomment-770205462, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAVJKKFIU2KPVP534K3PGLS4P3W7ANCNFSM4WCNCK7Q .

-- http://about.me/dominic.jones

[andymantell]

Ok let's try that again. I've released v5.0.0 since this is a breaking change really. Arguably the previous one should have been really but it's done now.

As per this commit... https://github.com/surevine/govuk-react-jsx/commit/c36f191f064969b59d5f9f782852f95ada2f524a ...the Error Summary no longer automatically focuses ever. It is entirely up to the calling app to decide when it is appropriate to do so.

Then, in govuk-react-jsx-examples, this is the change that you will need to make: https://github.com/surevine/govuk-react-jsx-examples/pull/1/files#diff-dad279dbcf6b8f7a902c5b582369bdb479d37f45e5d39919f16829e738718fb2 which will allow you to focus the error summary when someone has clicked the submit button.

You should be able to turn on dynamic validation and it will now update the error summary but not autofocus the error summary because of the formik.isSubmitting condition which means it only focuses when someone submits the form. If you want to validate on blur, you will need to add onBlur={formik.handleBlur} to each component.

The demo will remain with the dynamic onChange and onBlur validation turned off to match the way govuk prefers validation to be handled - but if you do turn it on it should no longer cause any focusing issues.

Hope this works ok for you.