[EuiAccordion] Allow interactive content within the buttonContent#5258
Merged
cchaos merged 15 commits intoelastic:masterfrom Oct 19, 2021
Merged
[EuiAccordion] Allow interactive content within the buttonContent#5258cchaos merged 15 commits intoelastic:masterfrom
buttonContent#5258cchaos merged 15 commits intoelastic:masterfrom
Conversation
added 7 commits
October 5, 2021 10:45
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5258/ |
added 2 commits
October 11, 2021 17:00
Contributor
Author
|
This PR is reviewable now with the caveat that I still have work to do on the rotation animation of the arrow button. |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5258/ |
added 2 commits
October 13, 2021 18:03
…e_nav_accordion # Conflicts: # src-docs/src/views/accordion/accordion.js # src-docs/src/views/accordion/accordion_form.js
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5258/ |
thompsongl
reviewed
Oct 18, 2021
Contributor
thompsongl
left a comment
thompsongl
left a comment
There was a problem hiding this comment.
Functional and a11y tested. LGTM
I still have work to do on the rotation animation of the arrow button.
Still working on this?
added 4 commits
October 18, 2021 16:19
…e_nav_accordion # Conflicts: # src-docs/src/views/collapsible_nav/collapsible_nav_all.tsx
Contributor
Author
|
Thanks for my own note reminder 😂 . Fixed. |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5258/ |
thompsongl
approved these changes
Oct 18, 2021
ym
pushed a commit
to ym/eui
that referenced
this pull request
Oct 29, 2021
…lastic#5258) * Make accordion arrows always EuiButtonIcons * Added `element` and `buttonElement` to customize HTML elements * Added `arrowProps` so collapsible group could change its color for dark * Hard coded example of stopPropagation
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is needed to allow our EuiCollapsibleNavGroup titles to navigate directly to Kibana's overview pages while keeping the collapse toggle on all the parts but the interactive content.
While looking through the component code, I saw a couple places where I could tell it was really old-gen code and could use an update.
1. Arrow icon
Before, we were displaying the arrow as just a simple EuiIcon, that we then wrapped in a
<button>when displayed on the right with anextraAction. Which we then applied custom interaction styles on top of. I replaced this logic by just always rendering an EuiButtonIcon but moving it out of thebuttonContentand removing from the tab-order if thebuttonContentis a<button>. (more on that later).It does mean that the focus outline is now only around the text of the button content, but it fixes the weird shape that sometimes happens and the arrow itself has consistent hover states to our normal icon buttons.
I also added
arrowProps?: Partial<EuiButtonIconProps>that gets passed to this button.2. Easily create a fieldset/legend combo with new
elementpropRight now this prop is purely restricted to
div | fieldset(div is default) , but can be expanded as we need. When the accordion is providedfieldset, it will automatically convert thebuttonContent's wrapping element to alegend. And updated the "Styled form form" section to use theelement = fieldsetsetting.This helps tremendously with VO when focusing into an input contained within an accordion.
3. Added
buttonElementto force to adivto allow for interactive contentRight now the only options are
'div' | 'legend' | 'button'. But when it is not abutton, it will force the arrow display as the focusable toggle for the accordion.I also updated the Collapsible Nav -> 'Full pattern with header and saved pins' example to better match what is being implemented in Kibana, including the "Add data" button at the bottom and updated "Manage deployment" button with the ghost Cloud icon. It's a good place to test the real-world "link inside the accordion header" functionality.
Checklist