Filter selection jumps to next entry on reset

[Kimmova]

Hi,

After typing the first letter into the search field the options are filtered and the first option is then correctly autoselected, so that it can (almost) be set by hitting the Enter key (for example).

However, after a few milliseconds, the next entry in the list is selected automatically. This happens continuously once the search input has been emptied and you start typing again.

The easiest way to reproduce this is through the official stackblitz link, where the problem also occurs:
https://stackblitz.com/github/bithost-gmbh/ngx-mat-select-search-example

Reproduction steps
To reproduce, start på selecting the first bank in the list under single selection (without any search filtering).
Next, type the letter 'b'.
Now it will select 'Bank B' even though 'Bank A' is the first to fulfill the criteria.
Now, use backspace to delete the entered search and type in the letter 'b' once again.
Now the selection will jump to 'Bank C' and so forth.

A gif of the behaviour can be seen here: https://gyazo.com/1f813102b2335ec537e20178b27f337b

The expected use case I have for using this in my app would is to quickly search for an entry and then hit Enter. With this behaviour it will always select the second option if more than one is present, given my search criteria.

Looking at the source code I cannot see anything that would directly cause this problem, which makes me think this might be an Angular Material issue? Can someone help shed some light on this?
I am just surprised there is no existing issues reported for this on this plugin.

It should also be noted that I have tried running this with the newest Angular and Material as well (7.0.1 at the time of writing), which has the same problem. The 'ngx-mat-select-search' version used for that setup is also the newest (1.4.1).

[macjohnny]

I think this is closely related to #67

[Kimmova]

It might be related yes, however it does briefly select the first entry correctly and then jumps one down.

Any idea what might be causing this?

[macjohnny]

I have no clue yet, but it could be related to mat select itself, maybe due to the frequent option list change.

[macjohnny]

If you could help and try to investigate this, i would appreciate it.

[Kimmova]

I got it to work in a development build by changing the _handleKeydown (mat-select-search.component.ts, l. 261) method to stop propagation on all keys except enter, up arrow and down arrow, since these are the only keys needed to pass to MatSelect as I see it.

Changed method to this:

handleKeydown(event: KeyboardEvent) {
    // only propagate control keys to MatSelect to prevent double selection
    if (event.keyCode !== 13 && event.keyCode !== 38 && event.keyCode !== 40) {
        event.stopPropagation();
    }
}

I do not see much point in passing anything other than these keystrokes, since this plugin will be handling the selection and filtering.
Any thoughts on this?

[macjohnny]

Thanks for having a look into this.
The Home/End/escape keys should be added to your solution, too. Probably there are even more keys that are handled by either the overlay panel, the mat select or the option.

I think it is better to filter some keys (space in this case) instead of passing only specific keys (home, arrow up/down, etc.), since this is more robust against changes of the mat select key down handling, see https://github.com/angular/material2/blob/985774a4eaa14d1dcbf1ad96ab176043d38f433e/src/lib/select/select.ts#L717

However, then it would be necessary to find out exactly what key is causing the problem and filter it, too.

What do you think?

[Kimmova]

I agree, the filter list I posted is incomplete and those keys that you mention should definitely be added as well.

While I also agree that only some keys should be filtered, the range of these keys might be all alphanumeric characters,

I think the issue originates in Material's ListKeyManager implementation, which is used in MatSelect. This is in order to provide a quicksearch in e.g. MatSelect so when you type alphanumeric characters consecutively, this first item with this combination will be set as active.
I believe this is being set here:
https://github.com/angular/material2/blob/master/src/cdk/a11y/key-manager/list-key-manager.ts#L152

Which is being triggered from MatSelect here:
https://github.com/angular/material2/blob/985774a4eaa14d1dcbf1ad96ab176043d38f433e/src/lib/select/select.ts#L739

And it looks like the filtering for which keys that cause this are listed here:
https://github.com/angular/material2/blob/master/src/cdk/a11y/key-manager/list-key-manager.ts#L236

It looks like this happens for all alphanumeric characters as far as I can see, so I think the solution would be to disable propagation for this range?

Any thoughts?

[Kimmova]

Also, in my tests I did not encounter problems with SPACE while in search field even if I excluded this from the stopPropagation list. Are you sure this check is necessary? I know that ENTER and SPACE will trigger selection, but it seems only ENTER has this effect while the search input is focused.

Once the select itself is in focus, both ENTER and SPACE will activate the item(s) regardless of this check, since it is another component.

If so, I would propose the following solution:

  _handleKeydown(event: KeyboardEvent) {
	// Prevent propagation for all alphanumeric characters
	if ((event.key && event.key.length === 1) || 
	    (event.keyCode >= A && event.keyCode <= Z) || 
	    (event.keyCode >= ZERO && event.keyCode <= NINE)) {
      event.stopPropagation();
    }
  }

[macjohnny]

@Kimmova thanks for the excellent analysis!
I think your proposed solution looks good.
Some remarks though:
You are checking capital A and capital Z, how about lowercases? Or are they already included with these constants? (I guess you want to use those from angular cdk)

The space key needed to be stopped from propagation, at least with the material version that was available when the initial version of ngx-mat-select-search was released. Maybe this has changed, as your tests suggest.

I would be glad if you could open a PR with your changes. I will then release a new version once I am back from holiday in about 3 weeks. Is that ok for you?

[Kimmova]

The javascript keycode for capital and non capital letters are the same, and yes it is from angular cdk, so this will not be a problem. Plus, on the majority of devices, it will be the event.key.length === 1 condition that will be met.

As far as I can tell, there is no (longer) need to stop propagation for the space key, tested in v 6.x and 7.x. Feel free to make a test yourself before you release, to confirm.

I will make a PR soon with the changes. I would like to have it included in my app's next release sometime next week, so I might just use my own fork or something until you release the update.

[maechler]

Fixed with release 1.4.2