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

Dropdown: Error occurred with keyboard action #1875

Closed
rokoroku opened this issue Jul 19, 2017 · 4 comments
Closed

Dropdown: Error occurred with keyboard action #1875

rokoroku opened this issue Jul 19, 2017 · 4 comments
Labels

Comments

@rokoroku
Copy link
Contributor

rokoroku commented Jul 19, 2017

Steps

For Dropdown without shorthanded options prop, we cannot move selections with keyboard arrows.

Expected Result

moves focus up & down

Actual Result

Dropdown.js?ac82:598 Uncaught TypeError: Cannot read property 'length' of undefined
    at Dropdown._this.moveSelectionBy (Dropdown.js?ac82:598)
    at HTMLDocument.Dropdown._this.moveSelectionOnKeyDown (Dropdown.js?ac82:213)
Dropdown._this.moveSelectionBy @ Dropdown.js?ac82:598
Dropdown._this.moveSelectionOnKeyDown @ Dropdown.js?ac82:213

Version

0.70.0

Testcase

https://codepen.io/rokoroku/pen/weLWXX

@layershifter layershifter changed the title Error occurred in Dropdown with keyboard action Dropdown: Error occurred with keyboard action Jul 20, 2017
@levithomason
Copy link
Member

As noted in the Dropdown docs:

Dropdown state is not fully managed when using the subcomponent API. The shorthand props API fully manages state...

We should fix the error, however, the fix will only prevent the error from being thrown. We will not attempt to parse children and update the state accordingly. In the future, we will likely drop all support for children in the Dropdown and require only the use of shorthand props.

@layershifter
Copy link
Member

layershifter commented Jul 21, 2017

I fully agree about drop of support children. BTW, I also have an idea to split Dropdown to multiple components like SearchDropdown and MultipleDropdown. Not sure that it's the best possible idea, but it will allow to make Dropdown more simple.

@rokoroku
Copy link
Contributor Author

I've used subcomponent API to use <Dropdown.Divider/>.
If it will be dropped, there should be shorthanded props for dropdown components such as Header, Divider, etc. (Or is there any options exists? it's not documented well.)

@levithomason
Copy link
Member

It is in progress, see #1724 for the issue/discussion and #1757 for the PR. Related to this is supporting nested sub menus with shorthand, #1365.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants