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] support append and prepend for group labels #7800

Conversation

mgadewoll
Copy link
Contributor

@mgadewoll mgadewoll commented May 30, 2024

Summary

closes #7798

This PR updates EuiComboBox to support the options props append and prepend to be output for group label options with isGroupLabel=true. This aligns the behavior with EuiSelectable.

example usage
Screenshot 2024-05-30 at 12 24 27

Changes

  • ensures append and prepend are output for group labels
  • updates group label styles to ensure proper layouts (support flex layout to allow for positioning appened content to be placed on the right of the panel)
  • adds group stories for EuiComboBox and EuiSelectable
  • adds/updates VRT reference images

QA

General checklist

  • Browser QA
    • Checked in both light and dark modes
    • Checked in mobile
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
  • Code quality checklist~
  • Release checklist
    • A changelog entry exists and is marked appropriately.
    • If applicable, added the breaking change issue label (and filled out the breaking change checklist) Designer checklist
    • Updated the Figma library counterpart

@mgadewoll mgadewoll marked this pull request as ready for review May 30, 2024 12:17
@mgadewoll mgadewoll requested a review from a team as a code owner May 30, 2024 12:17
Comment on lines +128 to +129
append: (
<EuiFlexItem css={{ alignItems: 'flex-end' }}>(append)</EuiFlexItem>
Copy link
Member

@cee-chen cee-chen May 30, 2024

Choose a reason for hiding this comment

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

Hmmm that's odd, the append nodes shouldn't need extra CSS from consumers to get them to align to the right 🤔 Is this only happening for group labels? We may need to add some extra CSS in EUI if so

Copy link
Contributor Author

@mgadewoll mgadewoll May 30, 2024

Choose a reason for hiding this comment

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

Yes, currently this is "needed". The passed append/prepend is output as is. There is no further styling or layout handling added. (code here)

If we only pass the flex item it'll grow but it's left aligned
Screenshot 2024-05-30 at 17 16 58

Copy link
Member

Choose a reason for hiding this comment

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

Ahhh gotcha. I wonder if we should add more OOTB styling for this? Probably an out of scope question for this PR, I'll ponder it more later / during either the Emotion conversion for this component or the EuiComboBox<>Selectable work!

Copy link
Member

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

✨ Thanks for the fast turnaround on this! We'll definitely need to figure out a way for keyboard users to reach interactive buttons inside group labels for ComboBoxes particularly, since tabbing won't work. We may have to talk to Trevor about that once he gets back!

@mgadewoll
Copy link
Contributor Author

mgadewoll commented Jun 1, 2024

✨ Thanks for the fast turnaround on this! We'll definitely need to figure out a way for keyboard users to reach interactive buttons inside group labels for ComboBoxes particularly, since tabbing won't work. We may have to talk to Trevor about that once he gets back!

Indeed, adding anything interactive on prepend/append breaks the accessibility of those added elements as they won't be reachable via keyboard (and only partially via manual screen reader navigation (but users won't know and expect anything to be there so they wouldn't navigate to it in any case).

@mgadewoll mgadewoll enabled auto-merge (squash) June 1, 2024 08:55
@kibanamachine
Copy link

Preview staging links for this PR:

@mgadewoll mgadewoll merged commit e92e996 into elastic:main Jun 1, 2024
5 checks passed
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cee-chen added a commit to elastic/kibana that referenced this pull request Jun 3, 2024
`v94.5.2` ⏩ `v94.6.0`

[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)

---

## [`v94.6.0`](https://github.com/elastic/eui/releases/v94.6.0)

- Updated `EuiComboBox` to support rendering `option.append` and
`option.prepend` in group labels
([#7800](elastic/eui#7800))

**Accessibility**

- Improved the accessibility experience of `EuiBetaBadge`
([#7805](elastic/eui#7805))
rohanxz pushed a commit to honeyn303/kibana that referenced this pull request Jun 4, 2024
`v94.5.2` ⏩ `v94.6.0`

[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)

---

## [`v94.6.0`](https://github.com/elastic/eui/releases/v94.6.0)

- Updated `EuiComboBox` to support rendering `option.append` and
`option.prepend` in group labels
([elastic#7800](elastic/eui#7800))

**Accessibility**

- Improved the accessibility experience of `EuiBetaBadge`
([elastic#7805](elastic/eui#7805))
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] support for append and prepend on group labels
4 participants