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

[EuiComboBox] Adding hit enter badge #3782

Merged
merged 10 commits into from
Jul 23, 2020

Conversation

miukimiu
Copy link
Contributor

@miukimiu miukimiu commented Jul 22, 2020

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

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

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

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

1 similar comment
@kibanamachine
Copy link

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

Copy link
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

…ons_list.tsx

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

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

@miukimiu miukimiu mentioned this pull request Jul 22, 2020
5 tasks
@kibanamachine
Copy link

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

@miukimiu
Copy link
Contributor Author

@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.

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

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

Copy link
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.

src-docs/src/views/combo_box/virtualized.js Show resolved Hide resolved
@kibanamachine
Copy link

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

@miukimiu miukimiu merged commit 9e08564 into elastic:master Jul 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants