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: Icon isn't displayed in selection #1147

Closed
rbideau opened this issue Jan 11, 2017 · 13 comments · Fixed by #4003
Closed

Dropdown: Icon isn't displayed in selection #1147

rbideau opened this issue Jan 11, 2017 · 13 comments · Fixed by #4003

Comments

@rbideau
Copy link

rbideau commented Jan 11, 2017

Steps

  1. Create a select Dropdown with options with text and icon
  2. Select an option

Expected Result

The icon and text should be displayed as the selected option

Actual Result

Only the text is displayed , not the icon

Version

v0.63.5

Testcase

semantic-ui codepen
semantic-ui-react codepen

@levithomason
Copy link
Member

Workaround

I'm a little torn on which way this should go. A Dropdown option (Dropdown.Item) can define both text and content. The item's text value is displayed in the Dropdown when the item is active. The content is used to show one value in the menu and a different value in the Dropdown when the item is active, see http://react.semantic-ui.com/modules/dropdown#item-content. So, to add text and an icon to the Dropdown for an active item, the item's text needs to contain both:

const options = [
  { value: '0', text: <span><Icon name='man' /> Man</span> },
  { value: '1', text: <span><Icon name='woman' /> Woman</span> },
  { value: '2', text: <span><Icon name='genderless' /> Other</span> },
]

This will render what you are looking for. Forked example http://codepen.io/levithomason/pen/apNOZp.

Proper fix

That said, I think your use case is intuitive and should probably work. What we'd need to do is render the children of the active item instead of the item's text prop only. This way, how ever the item ended up rendering in the menu is the how it will render in the Dropdown when active.

This approach will need to consider the fact that the content should not be rendered in the Dropdown. Also, if there are multiple children, then the item's children will be an array that we'll need to wrap with another element (i.e. div).

Likely, the proper implementation here will be to take the active item's props, remove the content and children, render a new DropdownItem with said props, and take its children. This way, we get the proper children given the DropdownItem's props configuration. The logic for choosing the text would need to handle something like:

const { children, content, ...rest } = activeItem.props
const _text = DropdownItem.create(rest).props.children

@rbideau
Copy link
Author

rbideau commented Jan 12, 2017

Thank you for your quick feedback.

Your workaround does works but generates the following warnings :

Warning: Only strings and numbers are supported as <option> children
Warning: validateDOMNesting(...): <span> cannot appear as a child of <option>. See DropdownParent > Dropdown > option > span.

I will try to put together a fix as you suggested and then open a pull request.

@levithomason
Copy link
Member

Thanks much. Note that the prop warnings are nonconsequential. However, we certainly want to fix them :)

@rbideau
Copy link
Author

rbideau commented Jan 13, 2017

I have some trouble implementing this.

It's looks like in this code:

const _text = DropdownItem.create(rest).props.children

children is undefined. I Think it might be due to React lifecycle and the fact that the DropdownItem has never been renderered.

I don't want to duplicate some of the logique from DropdownItem but I don't the any other options.

@levithomason
Copy link
Member

If you'd like to open a PR it may get more attention since there will be some code to review / test.

@stale
Copy link

stale bot commented Feb 4, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 30 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 4, 2018
@stale stale bot closed this as completed Mar 6, 2018
@levithomason
Copy link
Member

Thanks to @rijk, we have improved how we handle stale issues, see #2761. Reopening.

@levithomason levithomason reopened this May 14, 2018
@stale stale bot removed the stale label May 14, 2018
@stale
Copy link

stale bot commented Aug 12, 2018

There has been no activity in this thread for 90 days. While we care about every issue and we’d love to see this fixed, the core team’s time is limited so we have to focus our attention on the issues that are most pressing. Therefore, we will likely not be able to get to this one.

However, PRs for this issue will of course be accepted and welcome!

If there is no more activity in the next 90 days, this issue will be closed automatically for housekeeping. To prevent this, simply leave a reply here. Thanks!

@stale stale bot added the stale label Aug 12, 2018
@ThiefMaster
Copy link

Is there any way to get a visible icon while remaining searchable? As soon as I provide an element as text I can't search it anymore.

@stale
Copy link

stale bot commented May 20, 2019

There has been no activity in this thread for 180 days. While we care about every issue and we’d love to see this fixed, the core team’s time is limited so we have to focus our attention on the issues that are most pressing. Therefore, we will likely not be able to get to this one.

However, PRs for this issue will of course be accepted and welcome!

If there is no more activity in the next 180 days, this issue will be closed automatically for housekeeping. To prevent this, simply leave a reply here. Thanks!

@stale stale bot added the stale label May 20, 2019
@wendyperalta
Copy link

I am also interested in a fix for this.

@mfen
Copy link

mfen commented Mar 22, 2022

@layershifter thanks for the Flag and Image support, but I think Icon support was lost in the last commit of #4003. Was this intentional or is there a chance for a fix?

@layershifter
Copy link
Member

@layershifter thanks for the Flag and Image support, but I think Icon support was lost in the last commit of #4003. Was this intentional or is there a chance for a fix?

Can you please create an issue for the problem that you mentioned? 🙏

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