Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes left arrow key in dropdown search input #3596

Closed
wants to merge 1 commit into from
Closed

Fixes left arrow key in dropdown search input #3596

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 21, 2016

For the case where no item is selected. Because in this case $parentMenu is still something else than $menu.

For the case where no item is selected. Because in this case $parentMenu is still something else than $menu.
@jlukic jlukic added this to the 2.1.9 milestone Jan 25, 2016
@jlukic
Copy link
Member

jlukic commented Feb 2, 2016

Can you explain what might trigger this issue?

@jlukic jlukic modified the milestones: 2.2.x, 2.1.9 Feb 15, 2016
@jlukic
Copy link
Member

jlukic commented Feb 15, 2016

@sanjo Delaying this without a specific test case

@ghost
Copy link
Author

ghost commented Feb 19, 2016

Reproduce:

  1. Go to the "Search Selection" example on http://semantic-ui.com/modules/dropdown.html.
  2. Type "foo" into the search input
  3. Press the left arrow key to change the cursor position in the text. This currently doesn't work and this fix fixes that.

This fix just adds the condition:

if(hasSelectedItem) {
  ...
}

which is necessary because isSubMenuItem has the value true even when no item is selected, which interprets the left arrow key press wrongly as menu navigation action, instead of the default cursor position change action.

@jlukic
Copy link
Member

jlukic commented Feb 28, 2016

Woah that is bad.. thanks for the catch

@jlukic
Copy link
Member

jlukic commented Feb 28, 2016

Thanks for spotting this, this is a pretty heinous bug.

I've manually added the fix instead of merging due to conflicts when merging into the next branch. (See readme for details, but generally all PR should be merged into next branch)

jlukic added a commit that referenced this pull request Feb 28, 2016
@jlukic jlukic closed this Feb 28, 2016
@jlukic jlukic modified the milestones: 2.1.9, 2.2 May 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants