Add check mark to singleSelection EuiComboBox

[andreadelrio]

Summary

This PR adds a check mark to the singleSelection combo box. To do this I enabled showIcons in EuiFilterSelectItem when singleSelection is true and hid the cross icon using CSS.

• Checkbox in combo box with renderOption
  

Fixes #2808

Checklist

- [ ] Check against all themes for compatibility in both light and dark modes
- [ ] Checked in mobile

• Checked in IE11 and Firefox
  - [ ] Props have proper autodocs
  - [ ] Added documentation examples
• Added or updated jest tests
  - [ ] Checked for breaking changes and labeled appropriately
  - [ ] Checked for accessibility including keyboard-only and screenreader modes
• A changelog entry exists and is marked appropriately

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_2890/

[dimitropoulos]

@andreadelrio is there any chance I can convince you to wait on merging this until #2838 is in? I'd be happy to rebase this PR for you once #2838 is merged (which, could be tomorrow, maybe? It's pretty close, I think).

It'll be much easier to rebase this off of #2838 than to do the reverse.

[thompsongl]

@andreadelrio @dimitropoulos I agree that waiting is a good idea. We'll make sure the wait isn't long and that any conflicts are resolved for you

[cchaos]

@andreadelrio I'm confused by this one. Can you explain why the cross would show up in singleSelection combo boxes and why it'd be necessary to force hide it?

[andreadelrio]

Whoops, I was overcomplicating things, passing null to checked was enough. I made the changes.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_2890/

[cchaos]

Talked with @andreadelrio about this and the behavior is a little odd where the icon only shows on the currently focused item, not the currently selected item.

She's looking into it.

[andreadelrio]

@cchaos pushed changes based on your PR

I replaced let checked = activeOptionIndex === index ? 'on' : null; with let checked; as I think that comparison was unnecessary and we really only need the definition of checked we're doing inside the if:

          let checked;
          if (singleSelection) {
            checked =
              selectedOptions.length && selectedOptions[0].label === label
                ? 'on'
                : null;
          }

How things are working now:

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_2890/

[snide]

Just passing through. When in single selection mode this looks great 🎉 . My guess is we shouldn't leave room for the unused checks in multi-select mode.

Before

After

[andreadelrio]

Just passing through. When in single selection mode this looks great 🎉 . My guess is we shouldn't leave room for the unused checks in multi-select mode.

Before

After

@snide thanks for catching that! Just pushed a fix.

[cchaos]

I replaced let checked = activeOptionIndex === index ? 'on' : null; with let checked; as I think that comparison was unnecessary and we really only need the definition of checked we're doing inside the if

Ahh I didn't realize the original ternary was from this PR and not from master. 👍

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_2890/

[cchaos]

Looks great! I tested (Chrome only) in all the docs examples to make sure it works with the different rendering options and it does. Much better experience for single selection. 💯

[cchaos]

@andreadelrio Reminder though that we'll want to wait until the TS conversion of EuiComboBox PR #2838 gets merged first before merging this one.

[thompsongl]

^ I'm going to merge the TS conversion PR shortly and then resolve any conflicts here

[cchaos]

@thompsongl I don't think this is correct. off would mean the icon shows a cross instead of no icon. It should be undefined or on.

[cchaos]

[thompsongl]

Ah ok. Good catch

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_2890/

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_2890/