fix(select components): blur when selecting an option

[Mohammer5]

Description

SingleSelect did not blur when actually blurring the input when picking an option

---

Checklist

• API docs are generated
• Tests were added
• Storybook demos were added

[dhis2-bot]

🚀 Deployed on https://pr-1419--dhis2-ui.netlify.app

[cypress]

Passing run #3100 ↗︎

0

584

0

0

0

Details:

refactor: fix eslint issues
Project: ui	Commit: 1e7a1c96eb
Status: Passed	Duration: 06:46 💡
Started: Nov 9, 2023 9:18 AM	Ended: Nov 9, 2023 9:25 AM

Review all test suite changes for PR #1419 ↗︎

[ismay]

I did some testing on a normal select. Ours behaves a little differently.

On outside clicks

Normal select:

1. Click select -> it's focused, menu opens
2. Click outside of options (and outside of select) -> options close, still focused
3. Click outside of options (and outside of select) again -> select blurs

Our select:

1. Click select -> it's focused, menu opens
2. Click outside of options (and outside of select) -> options close, select blurs

On selection and outside click

Normal select:

1. Click select -> it's focused, menu opens
2. Click an option -> options close, still focused (but onFocus not called again)
3. Click outside of select -> select blurs

Our select:

1. Click select -> it's focused, menu opens
2. Click an option -> options close, still focused (and onFocus is called again)
3. Click outside of select -> onBlur not called

I think it's probably good to stay as close as possible to the normal select. Plus I would say it's clearly a bug that onBlur isn't called when clicking outside of the select after picking an option.

So if that's what we want:

1. Our select should close options if they're open on clicks outside of options, but not blur
2. It should blur on all clicks outside of the select if options is not open and the select has focus
3. onFocus should not be called again when selecting an option

I can imagine that 1. and 3. are out of scope for this PR (though still bugs and definitely related). Would you say that 2. matches with what you're trying to address with this PR?

[Mohammer5]

Although we've started dhis2/notes#345, this should get merged, too, so we get onBlur until we've implemented an accessible component

[Mohammer5]

Would cause a second call to onBlur

[Birkbjo]

Looks good to me. One questions though; is this consistent with how a native select works? Eg. it calls onBlur when selecting something - and not just when you move out of the select? I think this actually might be inconsistent for native across browsers?

[Mohammer5]

@Birkbjo

is this consistent with how a native select works? Eg. it calls onBlur when selecting something - and not just when you move out of the select? I think this actually might be inconsistent for native across browsers?

No, this is not how native selects work and it's basically impossible to replicate that (which is why @ismay and I started dhis2/notes#345). I've checked other implementations (e.g. the one that Joe linked to in the proposal) and they behave like the implementation in this PR

Native selects keep their focus when you select an option. Implementing something like that would involve workarounds of which I'm sure if they add more good than harm (e.g. when you select an option, which is outside of the button/input's element as it's in a layer/popper, the browser deselects that element. We'd have to manually reselect it again, which would trigger events that assistive technology would recognize).

[Birkbjo]

If this is just for validation to work, we could fix that in the FF component wrapper, right?

[Mohammer5]

@Birkbjo

If this is just for validation to work, we could fix that in the FF component wrapper, right?

We could, but why do it there if we can fix it in the underlying component instead?

[kabaros]

We could, but why do it there if we can fix it in the underlying component instead?

to avoid changing the behaviour for existing consumers I'd say - if a consumer had an onBlur already with logic that expects to run on clicking outside the select, it will now all of a sudden run on selecting an option. This would be acceptable if the fix was consistent with how native select works (I understand that's very hard to mimic though), but this is not the case - the fix is also a compromise.

I looked at the implementation that Joe linked to and I thinks it behaves similar to this PR because it passes the onBlur to both select and underlying option - does the native Select works similar to your PR if both select and option are passed the same onBlur handler?

Another question: From the consumer-side, trying to understand the validation issue you're trying to fix (feel free to link to a PR) - I take it that for the validation to work onBlur has to be triggered - not onChange?

[Mohammer5]

Another question: From the consumer-side, trying to understand the validation issue you're trying to fix (feel free to link to a PR) - I take it that for the validation to work onBlur has to be triggered - not onChange?

I gave it a try here: The onBlur is not called when you pass onBlur to an option.

---

@Birkbjo @kabaros

if a consumer had an onBlur already with logic that expects to run on clicking outside the select, it will now all of a sudden run on selecting an option.

Fair enough! I didn't think that far.. 🤦‍♂️ I guess with whatever we'll come up with here: dhis2/notes#345, we'll have a solution in the future and for now, I could probably even fix in the app itself (by calling RFF's onBlur when onChange is called). Should I close this PR? It'll be around for when someone else will encounter the same issue (in which case we could actually make this behavior only happen when a flag is set, e.g. blurOnChange)

[ismay]

I think that if we would want to proceed with this we should add tests to cover all the native select behaviour. Currently we're missing coverage for some of the behaviour I think. So we should test that:

On outside clicks

1. Click select -> it's focused, menu opens
2. Click outside of options (and outside of select) -> options close, still focused
3. Click outside of options (and outside of select) again -> select blurs

On selection and outside click

1. Click select -> it's focused, menu opens
2. Click an option -> options close, still focused (but onFocus not called again)
3. Click outside of select -> select blurs

Plus we should ensure we're not breaking existing API like onBlur's behaviour. Since the select is quite a complex component I think having the tests in place would help us ensure that we're getting the intended behaviour and not breaking any established API (unless necessary).

[ismay]

Since we might resolve this in a different way feel free to close. But I think the above is what I'd prefer if we're to continue with this PR.