Skip disabled items while navigating with keyboard #1823
Conversation
else if (ctrl.activeIndex < ctrl.items.length - 1) { | ||
var newIndex = ctrl.activeIndex + 1; | ||
while (_isItemDisabled(ctrl.items[newIndex]) && newIndex < ctrl.items.length) | ||
newIndex++; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we increment and compare in the while expression? To save repeating the condition inside it.
else if (ctrl.activeIndex > 0 || (ctrl.search.length === 0 && ctrl.tagging.isActivated && ctrl.activeIndex > -1)) { | ||
var newIndex = ctrl.activeIndex - 1; | ||
while (_isItemDisabled(ctrl.items[newIndex]) && newIndex >= 0) | ||
newIndex--; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the other comment, to save on duplication of checks
@Seprum this looks very useful! But it does need unit tests before it can be merged. 😥 Any chance you could add some? 😃 |
@Jefiozie you might like to pick this one up with tests etc. if you feel like it. I think it seems like a reasonable change. |
@user378230 , you want me to implement this fix? |
@Jefiozie Only if you want to! It looks fairly easy and makes sense, so thought you might like it 😝 (You can just open a new PR) |
@user378230 , see my PR 👍 |
Closing pr as handeld by other PR |
When user presses up/down keys within dropdown, disabled items become active too. It has no visual effect but still is a bit confusing. I have added check for disabled items insted of plain incrementing/decrementing
activeIndex
value.