Skip to content

Dropdown: support an array of string|element options #470

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

Closed
levithomason opened this issue Sep 6, 2016 · 3 comments
Closed

Dropdown: support an array of string|element options #470

levithomason opened this issue Sep 6, 2016 · 3 comments

Comments

@levithomason
Copy link
Member

levithomason commented Sep 6, 2016

Currently, the Dropdown takes an array of plain objects via the options prop. These objects are used to create the <Dropdown.Item /> children. Each object is spread as props on each item:

// Dropdown.js renderItems()

return _.map(options, (opt, i) => (
  <DropdownItem
    key={`${opt.value}-${i}`}
    active={isActive(opt.value)}
    onClick={this.handleItemClick}
    selected={selectedIndex === i}
    onMouseDown={e => e.preventDefault()}
    {...opt}
  />
))

We should also accept an array of strings or elements. An array of strings would be used as the item text and value. Whereas, an array of elements would be cloned.

We created src/factories for this exact purpose. They handle mapping a primitive value to props, spreading props objects, and cloning elements while merging className and spreading props. We should then use createFactory to create a createDropdownItem factory, see src/factories/index.js. We would then use the createDropdownItem factory in place of the _.map() callback used above.

@mikesparr
Copy link

mikesparr commented Nov 7, 2016

The docs state that Dropdown.Item onClick event is supposed to trigger func(event, value, text) but I see the text value as undefined and only ever see value. Is this incomplete or part of this request to enhance Dropdown? (I was trying to get the name and value of selected item but only ever get the value)

    handleItemClick = (e, value, text) => {
    console.log(value);
    console.log(text);
        } // displays value but text is `undefined`


        <Dropdown 
                trigger={<Icon name='settings'/>} 
                pointing='top right' 
                icon={null}>
          <Dropdown.Menu>
              <Dropdown.Item text='View' name='view' value={key} onClick={this.handleItemClick} />
              <Dropdown.Item text='Delete' name='delete' value={key} onClick={this.handleItemClick} />
          </Dropdown.Menu>
        </Dropdown>

@levithomason
Copy link
Member Author

@mikesparr Thanks, the docs are wrong and the method needs updated. See #805 for the fix. We'll pass all props back in place of value.

@levithomason
Copy link
Member Author

Closing this for housekeeping. Per my notes in the PR, I'm not sure this work is worth it. The gains are minor and the repercussions are major. We've also not had hardly any community interest here.

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

Successfully merging a pull request may close this issue.

2 participants