Skip to content

[EuiSelectable+EuiComboBox] recent regresion fixes#3784

Merged
thompsongl merged 4 commits intoelastic:masterfrom
myasonik:selectable-a11y/regression-fixes
Jul 22, 2020
Merged

[EuiSelectable+EuiComboBox] recent regresion fixes#3784
thompsongl merged 4 commits intoelastic:masterfrom
myasonik:selectable-a11y/regression-fixes

Conversation

@myasonik
Copy link
Copy Markdown
Contributor

@myasonik myasonik commented Jul 22, 2020

Summary

Fixes two regressions caused by the recent selectable-a11y branch.

  1. Adding space as a option for how to selection items removes the ability to have items with in them so reverting this change.
  2. With the new react-window library replacing react-virtualized, this in conjunction with EuiFormRow was causing an extra rerender on click of virtualized elements which broke clicking... Adding a check in EuiFormRow for if the thing needs to rerender fixes it.

Checklist

  • Checked in mobile
  • Checked in Firefox
  • Checked for breaking changes and labeled appropriately
  • Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately
    - [ ] Checked Code Sandbox works for the any docs examples
    - [ ] Added or updated jest tests
    - [ ] Props have proper autodocs
    - [ ] Added documentation
    - [ ] Checked in IE11
    - [ ] Check against all themes for compatibility in both light and dark modes

Copy link
Copy Markdown
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Changelog formatting nit-picks but otherwise this LGTM!

@kibanamachine
Copy link
Copy Markdown

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

Co-authored-by: Chandler Prall <chandler.prall@gmail.com>
@myasonik myasonik requested review from cchaos and chandlerprall July 22, 2020 17:59
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.

Tested for both original bugs which are both fixed! Thanks for the quick PR!

@kibanamachine
Copy link
Copy Markdown

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

Copy link
Copy Markdown
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

❤️

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.

5 participants