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.

Sign up for GitHub

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

Jump to bottom

Combobox: multiple fixes. #674

Closed
wants to merge 11 commits into from
Closed

Combobox: multiple fixes. #674

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b4e53a5
Fix broken placeholder.
brunano21 Nov 28, 2016
b0199b3
Fix Cancel handler for ComboPopup, which wasnt clearing eventual sele…
brunano21 Nov 29, 2016
ff26def
Do not forward selection event to the list if dropdown is not opened.
brunano21 Nov 30, 2016
23c5824
Fix autocomplete mode bug: when inputNode was empty and in mobile, th…
brunano21 Dec 1, 2016
80e6270
In not-autocomplete mode (hasDownArrow=true), set the popup's inputNo…
brunano21 Dec 2, 2016
3f36ca1
Refocus inputNode if user clicks on list's item.
brunano21 Dec 5, 2016
50a0edf
Focus any previously focus list's item when opening the dropdown.
brunano21 Dec 5, 2016
95a0fb2
Set aria-expanded=true|false attribute when the popup opens and close…
brunano21 Dec 6, 2016
4d46111
Fix broken readOnly (at creation time). Add tests.
brunano21 Dec 1, 2016
49fabb6
jshint fixes.
brunano21 Dec 13, 2016
1ff8cb7
Add aria-expanded and aria-describedby attributes for mobile version …
brunano21 Dec 15, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Combobox.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,8 @@ define([
}
}.bind(this), 100); // worth exposing a property for the delay?
}
} else {
this.focusNode.focus(); // put the focus back to the inputNode.
Copy link
Member

@wkeese wkeese Dec 30, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change should be tested (that only passes after your change to Combobox.js). Probably just updating one of the existing tests to check focus after clicking a list item.

Actually, I loaded cwithout your changes, and tried both the first and the second Combobox on that page, and it seems like clicking a list's item already refocuses the Combobox if autoFilter===false, or the Combobox's <input> if autoFilter===true??

Also, I'm confused about what happens when:

  1. The first Combobox in Combobox-decl.html where you can't focus the <input>. What happens then?
  2. Multi-select mode. In that case don't you want to keep focus on the list item?
  3. ComboPopup. Does this code run in the ComboPopup case, and if so, doesn't focus go to the ComboPopup's <input> or the original ComboBox <input>? And what if the ComboPopup isn't showing an <input>, like the first example in ComboPopup.html?

}
}.bind(this)),

Expand Down