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

[EuiSelectable] Add the single triggering option as a parameter to onChange #6487

Merged
merged 4 commits into from
Dec 15, 2022

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Dec 15, 2022

Summary

closes #6485

@Heenawter's team has a fairly custom include/exclude usage of EuiSelectable where they want to be able to quickly grab the triggering option without having to scrub through the event or the entire options array to figure out what changed.

This is an extremely reasonable request, just IMO, and is also a very quick add onto the existing onChange callback.

QA

General checklist

  • Checked in Chrome, Safari, Edge, and Firefox
  • Props have proper autodocs and playground toggles
  • Added or updated jest and cypress tests
  • Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately
  • Revert [REVERT ME] commit before merge

- [ ] Checked in both light and dark modes
- [ ] Checked in mobile
- [ ] Added documentation
- [ ] Checked Code Sandbox works for any docs examples
- [ ] Updated the Figma library counterpart

@cee-chen cee-chen requested a review from 1Copenut December 15, 2022 18:14
Comment on lines 49 to 51
onChange={(newOptions, event, changedOption) => {
setOptions(newOptions);
console.log({ newOptions, event, changedOption });
Copy link
Contributor Author

@cee-chen cee-chen Dec 15, 2022

Choose a reason for hiding this comment

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

@Heenawter Please let us know if this satisfies your use case. You can examine the output of changedOption at https://eui.elastic.co/pr_6487/#/forms/selectable#options-can-be-excluded (once staging is done spinning up in ~30 mins or so).

The returned changedOption should be the option obj that was clicked/toggled, with the checked status updated (either 'on', 'off', or undefined). Its various values should be present in this object.

Copy link
Contributor

Choose a reason for hiding this comment

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

@constancecchen This is amazing, thank you so much! I tested it out, and it looks like it's doing exactly what we would need! 🎉

@@ -450,6 +452,7 @@ export class EuiSelectableList<T> extends Component<EuiSelectableListProps<T>> {
event: EuiSelectableOnChangeEvent
) => {
const { onOptionClick, options, singleSelection } = this.props;
let changedOption = { ...addedOption };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Minor technical note/aside: cloning addedOption via spread operator is likely unnecessary here, but I wanted to be very careful not to mutate the option objects being passed to us by consumers no matter what, so this does the same thing const updatedOption = { ...option }; is doing below, just in case

@kibanamachine
Copy link

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

Copy link
Contributor

@1Copenut 1Copenut left a comment

Choose a reason for hiding this comment

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

👍 LGTM! Spot tested keyboard and screen readers locally.

@cee-chen cee-chen enabled auto-merge (squash) December 15, 2022 19:36
@kibanamachine
Copy link

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

@cee-chen cee-chen merged commit 784a553 into elastic:main Dec 15, 2022
@cee-chen cee-chen deleted the 6485 branch December 15, 2022 20:13
jbudz pushed a commit to elastic/kibana that referenced this pull request Dec 22, 2022
## Summary

`[email protected]` ⏩ `[email protected]`

---

## [`72.0.0`](https://github.com/elastic/eui/tree/v72.0.0)

- Added the `customQuickSelectRender` render prop to
`EuiSuperDatePicker`, which allows customizing the Quick Select popover
([#6382](elastic/eui#6382))
- `EuiFilePicker` styles have been updated to look more like an
interactive element. ([#6479](elastic/eui#6479))
- Added a third argument to `EuiSelectable`'s `onChange` callback. The
single `option` object that triggered the `onChange` event is now also
passed to consumers with its most recent `checked` state
([#6487](elastic/eui#6487))

**Bug fixes**

- `EuiTabs` now passes `size` and `expand` to all children using a React
context provider. ([#6478](elastic/eui#6478))
- Fixed security warnings caused by `[email protected]` sub-dependency
([#6482](elastic/eui#6482))

**Breaking changes**

- Removed `size` and `expand` props from `EuiTab`
([#6478](elastic/eui#6478))

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

**Deprecations**

- Renamed `EuiPageSideBarProps` to `EuiPageSideBarProps_Deprecated`, to
reduce usage/confusion with `EuiPageSidebar`
([#6468](elastic/eui#6468))

Co-authored-by: Kibana Machine <[email protected]>
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.

[EuiSelectable] Feature request to return single option value(s) on change/click
4 participants