Skip to content
This repository was archived by the owner on Oct 2, 2019. It is now read-only.

JS tagging error when using up arrow to highlight the option before the first in the list. #624

Closed
JeromeLam opened this issue Feb 1, 2015 · 3 comments

Comments

@JeromeLam
Copy link
Contributor

Here's a screenshot of the error in Chrome's developer tools:
image

To reproduce, just open the tagging demo, focus on any input field, make sure at least one option is highlighted in the dropdown list and use the up arrow key until the highlight goes through the top of the dropdown. I validated this against v0.9.6 and master. Note that there is no problem if you used the down arrow key to try and go through the bottom of the dropdown. The highlight stays stuck on the last item.

Thanks for looking into this and keep up the great work!

@JeromeLam JeromeLam changed the title JS error when using up arrow to highlight the option before the first in the list. JS tagging error when using up arrow to highlight the option before the first in the list. Feb 2, 2015
@JeromeLam
Copy link
Contributor Author

I've identified the potential cause for this error here: https://github.com/angular-ui/ui-select/blob/master/src/select.js#L554

I can't seem to figure out why the condition ctrl.search.length === 0 && ctrl.tagging.isActivated was added for KEY.UP in #63. If I haven't typed anything in $select.search, this allows activeIndex to become negative, leading to the dropdown highlight disappearing.

If anyone has any insight, please provide some feedback. I can then create a PR to rectify this. Thanks!

@brianfeister
Copy link

Thanks @JeromeLam this is related to a condition where tagging-label="false" suppresses the value in the dropdown so that you can do "free tagging" but have it feel more like "free search", omitting the My Tag (new) bit. This is how I'm using the directive in a combo / additive filter context. Index tracking is affected differently between this and the other iteration where the dropdown list includes the new value. If you submit a PR (again, would be awesome), make sure you test with tagging-label="{whatever}" but also tagging-label="false"

@JeromeLam
Copy link
Contributor Author

Hey @brianfeister, I created PR #638. I hope the code changes are aligned with your expectations of index tracking.

brianfeister pushed a commit that referenced this issue Feb 6, 2015
Issue/#624 - Add extra handling for negative activeIndex.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants