feat(Dropdown): support MenuItem and Button types

[Chris-R3]

Steps

I'm using a Dropdown as Menu.Item and noticed that the width of the menu item seems off. I checked the Semantic-UI docs (3rd example) and they are using an additional icon class for the menu item if the Dropdown only contains an Icon.

Expected

class="ui dropdown icon item"

Result

class="ui dropdown item"

Workaround

adding className='icon'as additional Dropdownprop

Testcase

http://codepen.io/Chlris-R3/pen/PGQbqZ

• left: without icon class
• right: using className='icon' as additional Dropdownprop

Is this a bug / missing feature / working as intended?

[levithomason]

Confirmed issue. We're lacking feature support here. This one is going to need some thought. The icon class is required only in limited circumstances. There are multiple Dropdown usages to consider here:

Menu Item

When a Dropdown is a Menu Item:

• only the item className is required
• the icon child must appear after the text child (regular Dropdowns have text then icon)
• if there is an icon and no text nor placeholder, then the icon className is required

<div class="ui dropdown icon item">

Button

When the Dropdown is a Button

• the icon child must appear after the text child (regular Dropdowns have text then icon)
• the icon className is required if there is an an icon with no text nor placeholder
• the icon className is required if there is an an icon with text and it is labeled
• the icon className is not required if the Dropdown button does not also have an icon

Labeled Icon Button Dropdown

<div class="ui floating labeled icon dropdown button">

Icon Button Dropdown

<div class="ui icon top left pointing dropdown button">

Conclusion

There are no features here that require augmentation. Using augmentation actually does not work with the Button as Dropdown use case. Since augmentation will not solve all cases, I suggest we solve these use cases with a different consistent approach.

We could handle this by adding an item prop to the Dropdown. We already have a button prop. Finally, we can fix the icon className issue by checking if the Dropdown is an item or button and has only an icon (no text or placeholder).

Whoever takes up this PR, we should also tackle #562 and add the missing Dropdown examples. Adding the examples alone will assist in working these use cases out.

[levithomason]

Update:

• missing dropdown examples have been added.
• the item prop has been merged, for use in menus:<Dropdown item />
• basic button prop support exists and is documented: <Dropdown button />

TODO:

• logic for the icon className, see "Conclusion" in my last comment above

Also note, many of the icon button fixes in the Dropdown docs are being completed in #1038.

[pedrocostadev]

Hi guys,

I would like to fix that. I'm planning add the icon class if Dropdown is an item and have no text nor placeholder (how you refer above in conclusion).
I'm plan do this on Dropdown.js file. Also, do you think would be usefully if I add a little example of this to the section?

[levithomason]

Definitely! You'll find there are TODOs already in Dropdown.js as well as Dropdown-test.js, I believe. We also have Examples already made that will just need updated.

[pedrocostadev]

Ok let's do it

[pedrocostadev]

So, according rules above my plan is :

const isItem = item && item.name === 'MenuItem'
const haveNoLabelAndPlaceHolder = !this.props.text && !this.props.placeholder
const isButton = !_isNil(button)
const haveTextAndIsLabeled = this.props.text && labeled
const needIconClassName = (isButton && haveTextAndIsLabeled) || (isItem && haveNoLabelAndPlaceHolder)

...
const classes = cx(
    ...
    needIconClassName && useKeyOnly(icon, 'icon'),
    ...
)

@levithomason do you think is correct? May be I can put this logic in new or existing file on lib folder?
To show this i also plan create to a file called 'DropdownExampleIconMenuItem.js' in 'app/Examples/modules/Dropdown/Types/'.

Do you agree?

[levithomason]

@pedrocostadev Sorry for the extreme delay. Yes, this approach sounds sane. How about we start a PR and iterate there instead? We're very close to shipping this one!

[stale]

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.

[levithomason]

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

[stale]

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]

This issue will be closed due to lack of activity for 12 months. If you’d like this to be reopened, just leave a comment; we do monitor them!

[stale]

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!

[Ranzeb]

I am new to open-source contributions and I want to contribute. Can I handle this issue?

[HemanthPaila]

Hey, @Chris-R3 I would like to work on this issue. Could you please assign it to me?