-
Notifications
You must be signed in to change notification settings - Fork 841
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
[EuiAccordion] Allow external control #3614
Conversation
Preview documentation changes for this PR: https://eui.elastic.co/pr_3614/ |
Changes look good, though thinking about
Thoughts on calling |
I could definitely go either way on this. |
I think so; let's go that way, then this is good to merge |
Preview documentation changes for this PR: https://eui.elastic.co/pr_3614/ |
Looks like the accordion_forceState.js example wasn't updated and expects the inverse value in |
Preview documentation changes for this PR: https://eui.elastic.co/pr_3614/ |
jenkins test this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes LGTM! Tested in the PR's docs
Preview documentation changes for this PR: https://eui.elastic.co/pr_3614/ |
Summary
Resolves #3594 by allowing
forceState
andonToggle
to work together as an external state value and update function pair.When
forceState
is in useonToggle
will be called with a boolean reflecting the current open/closed state. This might be slightly confusing as standardonToggle
is called with a boolean reflecting the new open/closed state after click.Checklist
- [ ] Check against all themes for compatibility in both light and dark modes- [ ] Checked in mobile- [ ] Checked in IE11 and Firefox- [ ] Checked for accessibility including keyboard-only and screenreader modes