A Fix for Issue #286 — Spacebar input not registered when 'No results found'

[AdenFraser]

This is a proposed fix for Issue #286

The handleSpace method is used to either select a focused option when one exists, or open the full list of options when it is not open. However, there is an edge-case where the user is attempting to enter a string which includes a space, which is not in the list. The space-key entry is omitted on first try, resulting in a malformed string.

This fix simple checks whether there are available results/'No results found' message (or equivalent) is showing — and if that is the case, there is no need to re-attempt to open the menu, thus allowing the user to input their string as expected.

Fixes #286

[36degrees]

I think we can use menuIsVisible instead of menuOpen here, which I think better captures the intent and avoids adding another expression?

[AdenFraser]

That makes a lot more sense. Sorry am new to this package and codebase, didn't see the menuIsVisible method!

I'll get that amended and pushed.

[36degrees]

No worries – I'm still getting to grips with it myself!

[AdenFraser]

Actually I'm not too sure how best to refactor this, as the menuIsVisible const is insider the render()?

Making menuIsVisible available from handleSpace will also require const showNoOptionsFound = this.props.showNoOptionsFound && inputFocused && noOptionsAvailable && queryNotEmpty && queryLongEnough from the render() too? Which then relies on the latter four also being available.

Unless I'm overlooking something — it's mighty hot in our office! 😅💦☀️

[36degrees]

Sorry Aden, you're quite right (I told you I was still getting to grips with it myself!)

From talking to @joelanman who originally added this feature, the intent is that when you focus the element using the keyboard (i.e. by tabbing to it) and press space it should open the menu and display all results (mimicking behaviour of native selects).

I've just tested your fix and whilst it corrects the behaviour that you're seeing (adding a space whilst seeing no results triggers the menu), it doesn't quite work in that scenario – this.state.options.length is 0 if there is no query, so this handler doesn't kick in and rather than opening the menu it just searches for all items with a space in the name.

I think it makes sense to open the menu if it is not already open and there is not already a query – which would work in both cases I think? In which case changing the line to…

if (this.props.showAllValues && this.state.menuOpen === false && this.state.query != '') {

might make sense? I've done some initial testing and it seems to work well, but it'd be good if you could double-check that solves your problem.

[AdenFraser]

Hi, gave that a go but that didn't work — although I think we actually just need this.state.query === '', since we only wish for the options list to appear when handling space input initially, not with latter space input.

I've tested it with both the intended original scenario and against the issue I was facing and both now seem to be resolved.

Have rebuilt dist, rebased and squashed and all pushed up.

Aden

[36degrees]

Amazing, thanks Aden! I think that's what I intended to suggest, not sure how I messed that up.

[36degrees]

Thanks for this Aden! I've had a little play locally and your fix does solve the problem, but I've got a suggestion I think makes it slightly clearer.

[36degrees]

Looks good. Needs a second approval from someone else on the team.

[kr8n3r]

thanks @AdenFraser for fixing it :)