Skip to content

[EuiComboBox] Adding hit enter badge#3782

Merged
elizabetdev merged 10 commits intoelastic:masterfrom
elizabetdev:combo-box-hit-enter
Jul 23, 2020
Merged

[EuiComboBox] Adding hit enter badge#3782
elizabetdev merged 10 commits intoelastic:masterfrom
elizabetdev:combo-box-hit-enter

Conversation

@elizabetdev
Copy link
Copy Markdown
Contributor

@elizabetdev elizabetdev commented Jul 22, 2020 •
edited
Loading

Summary

This PR introduces a hit enter badge on the right side of a EuiComboBox option. The idea of the badge is to help users to know that they can press enter to add an option.

The idea of adding this badge started on #3702.

  • The badge appears when focusing an option
  • The badge appears on two empty states: Add {searchValue} as a custom option. I also added the badge to Add each item separated by {delimiter}" but I can't test this (Is this code exception done properly?)
  • Instead of using an icon for the badge, I'm using an HTML character entity.

Screenshot 2020-07-22 at 14 06 26

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
  • [ ] Checked Code Sandbox works for the any docs 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
Copy link
Copy Markdown

kibanamachine commented Jul 22, 2020

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

@elizabetdev elizabetdev marked this pull request as ready for review July 22, 2020 12:29
@elizabetdev elizabetdev requested a review from cchaos July 22, 2020 12:29
@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Jul 22, 2020

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

1 similar comment
@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Jul 22, 2020

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

cchaos
cchaos suggested changes Jul 22, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also added the badge to Add each item separated by {delimiter}" but I can't test this (Is this code exception done properly?)

Yeah I'm seeing that there is a bug that where you can paste a delimited list and if you click outside of the combobox it will create each option properly, but simply hitting enter only take the last item. Is this something you can look into?

It seems like there's extra space below the custom option text.

Screen Shot 2020-07-22 at 10 05 50 AM

Can you run through some of the ComboBox examples to make sure their settings are appropriate? For example, the Virtualized example has a custom placeholder saying "Select or create options" but that example doesn't have the ability to add a custom option.

Screen Shot 2020-07-22 at 10 04 04 AM

Be sure not to show the badge on disabled items

Screen Shot 2020-07-22 at 10 17 09 AM

@elizabetdev @cchaos
Update src/components/combo_box/combo_box_options_list/combo_box_opti…
dae7797 
…ons_list.tsx

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Jul 22, 2020

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

@elizabetdev elizabetdev mentioned this pull request Jul 22, 2020
Merged
5 tasks
@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Jul 23, 2020

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

@elizabetdev
Copy link
Copy Markdown
Contributor Author

elizabetdev commented Jul 23, 2020

@chaos I fixed the issues you pointed out:

  • Removed extra space bellow Add add {searchValue} as a custom option
  • Looked into the examples to make sure the placeholders are ok
  • Removed the badges on disabled options
  • Added an icon inside the badge. I’m waiting for Add returnKey glyph to EuiIcon #3783 to be merged to have the right icon.
  • The underlining on hover problem is not happening anymore because we don’t have a text inside the badge
  • Changed to spans the elements getting rendered inside the button.
  • There was also a EuiFlexItems that was part of the EuiFilterSelectItem I changed to a span.
  • I'm going to create an issue for the delimiter bug.

@elizabetdev elizabetdev requested a review from cchaos July 23, 2020 13:46
@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Jul 23, 2020

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

cchaos
cchaos approved these changes Jul 23, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM. Tested in Chrome, Safari, and FF. I also tested the filter group buttons to make sure they were rendering properly with the span. Just waiting on that returnKey icon, then this is good to go.

@kibanamachine
Copy link
Copy Markdown

kibanamachine commented Jul 23, 2020

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

@elizabetdev elizabetdev merged commit 9e08564 into elastic:master Jul 23, 2020
@thompsongl thompsongl mentioned this pull request Jul 31, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@cchaos cchaos cchaos approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@elizabetdev @kibanamachine @cchaos