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] Added option.prepend and option.append support #6953

Merged
merged 11 commits into from
Jul 21, 2023

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Jul 17, 2023

Summary

closes #2358 (see linked issue comments for more context as to why this particular API was chosen above others)

This PR adds the ability for consumers to pass in prepend and append nodes to EuiComboBox's options, e.g.

<EuiComboBox
  options={[
    {
      label: 'Hello',
      prepend: <EuiIcon type="faceHappy" size="s" />,
    },
    {
      label: 'World',
      append: <EuiIcon type="faceSad" size="s" />,
    },
  ]}
/>

These nodes are rendered for both in the dropdown options as well as in the selected pill display:

Screenshots

Multi selection:


Single selection:

QA

General checklist

  • Checked in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • Props have proper autodocs (using @default if default values are missing) and playground toggles
  • Added documentation
    - [ ] Checked Code Sandbox works for any docs examples
  • Added or updated jest and cypress tests
    - [ ] Checked for breaking changes and labeled appropriately
  • Checked for accessibility including keyboard-only and screenreader modes
    • Note that screen reader UX isn't great on EuiComboBox, but we already knew this / were going to switch to dogfooding EuiSelectable
  • A changelog entry exists and is marked appropriately
    - [ ] Updated the Figma library counterpart

@shahzad31 shahzad31 requested a review from cee-chen July 17, 2023 12:00
@kibanamachine
Copy link

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

@shahzad31 shahzad31 requested a review from a team July 17, 2023 15:20
@kibanamachine
Copy link

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

@cee-chen cee-chen self-assigned this Jul 17, 2023
@kibanamachine
Copy link

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

@cee-chen
Copy link
Member

Hey Shahzad! Super apologies it's taken me so long to get around to reviewing this PR - I plan on taking some time to look at it first thing tomorrow.

Any objections if I push up any changes/cleanup/feedback directly to your branch? If you'd prefer that I leave code comments in GitHub instead, no worries, just let me know.

@shahzad31
Copy link
Contributor Author

@cee-chen for sure feel free to push updates.

@cee-chen cee-chen changed the title [ComboBox] Added icon option [EuiComboBox] Added option.prepend and option.icon support Jul 19, 2023
+ cleanup - flatten more `euiComboBoxOption` selectors
+ other misc cleanup - remove unnecessary eslint disable (accounts for rest spread), add some newlines
- Use just one demo that switches between single selection and multi selection

- Reuse as much copy from `EuiSelectable` as possible

- misc cleanup - remove `Fragment`s
@cee-chen cee-chen changed the title [EuiComboBox] Added option.prepend and option.icon support [EuiComboBox] Added option.prepend and option.append support Jul 19, 2023
@kibanamachine
Copy link

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

@cee-chen
Copy link
Member

@ryankeairns Do you mind taking a designer pass on https://eui.elastic.co/pr_6953/#/forms/combo-box#option-rendering and letting me know if the UI (spacing, etc.) looks good to you or if you have any changes you'd like to see? Worth mentioning, the pill display and dropdown option display can be styled separately.

@ryankeairns
Copy link
Contributor

It appears the content is overflowing the pill, see below:

CleanShot 2023-07-20 at 13 39 50@2x

With only a quick look, the culprit seems to be the line-height on .euiComboBoxPill. Decreasing it from 22 to 20 looks like the desired end result.

CleanShot 2023-07-20 at 13 42 05@2x

@cee-chen
Copy link
Member

Great eye Ryan!! I was wondering why the vertical alignment was slightly bugging me, I just thought I was going crazy. Out of curiosity, is this already an issue on main/prod?

@kibanamachine
Copy link

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

@ryankeairns
Copy link
Contributor

Great eye Ryan!! I was wondering why the vertical alignment was slightly bugging me, I just thought I was going crazy. Out of curiosity, is this already an issue on main/prod?

I don't believe so. It looked to come directly from this new option.append text... the (5)

@cee-chen cee-chen merged commit 0fc3700 into elastic:main Jul 21, 2023
1 check passed
@shahzad31 shahzad31 deleted the pills branch July 27, 2023 18:41
mistic pushed a commit to elastic/kibana that referenced this pull request Aug 3, 2023
`85.0.0` ➡️ `85.1.0`

⚠️ The biggest change in this PR by far is the **removal** of several
`EuiFilterSelectItem` CSS classes and styles associated with those
classes. EUI has previously exported component-specific CSS classes for
usage such as `.euiFilterSelect__items`,
`.euiFilterGroup__popoverPanel`, or `.euiAccordionForm`.

❌ **As of our move to CSS-in-JS, we are actively moving away from this
architecture**. The goal is to either provide either a component or prop
directly to you instead of a CSS class. As a reminder, our classNames
are not guaranteed APIs and are subject to change without warning -
should you need to tweak or customize EUI's styling, we strongly
recommend passing in your own `className`.

💬 Moving forward, EUI will attempted to convert any usages of removed
CSS classes and their direct usages in Kibana for you. In most cases,
we'll hopefully be able to take the correct path of using a component or
prop instead. In cases where we cannot or significant/complex changes
are required, we may temporarily convert the CSS to a static CSS-in-JS
usage instead and add a TODO asking the relevant team to address this in
their own time (for example, the deprecation of `EuiFilterSelectItem`
and its conversion to `EuiSelectable`).

## [`85.1.0`](https://github.com/elastic/eui/tree/v85.1.0)

- Updated `EuiComboBox`'s `options` to accept `option.append` and
`option.prepend` props
([#6953](elastic/eui#6953))
- Updated deprecated `.substr()` usages to `.substring()`
([#6954](elastic/eui#6954))
- Updated `EuiInlineEdit`'s read mode button to include a title tooltip,
increasing readability of truncated text
([#6966](elastic/eui#6966))

**Bug fixes**

- Fixed `EuiFilterGroup`'s responsive styles
([#6983](elastic/eui#6983))

**Deprecations**

- Deprecated `EuiFilterSelectItem`; Use `EuiSelectable` instead
([#6982](elastic/eui#6982))

**CSS-in-JS conversions**

- Converted `EuiFilterSelectItem` to Emotion
([#6982](elastic/eui#6982))
- Removed `.euiFilterSelect__items` CSS; Use `EuiSelectable` instead
([#6982](elastic/eui#6982))
- Removed `.euiFilterSelect__note` and `.euiFilterSelect__noteContent`
CSS; Use `EuiSelectableMessage` instead
([#6982](elastic/eui#6982))
- Added `focus.transparency` and `focus.backgroundColor` theme tokens
([#6984](elastic/eui#6984))

---------

Co-authored-by: Cee Chen <constance.chen@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
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.

[EuiComboBox] Allow custom render of selected items
4 participants