-
Notifications
You must be signed in to change notification settings - Fork 839
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 interactive content within the buttonContent
#5258
[EuiAccordion] Allow interactive content within the buttonContent
#5258
Conversation
Preview documentation changes for this PR: https://eui.elastic.co/pr_5258/ |
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/ |
…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/ |
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.
Functional and a11y tested. LGTM
I still have work to do on the rotation animation of the arrow button.
Still working on this?
…e_nav_accordion # Conflicts: # src-docs/src/views/collapsible_nav/collapsible_nav_all.tsx
Thanks for my own note reminder 😂 . Fixed. |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5258/ |
…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
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 thebuttonContent
and removing from the tab-order if thebuttonContent
is 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
element
propRight 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 = fieldset
setting.This helps tremendously with VO when focusing into an input contained within an accordion.
3. Added
buttonElement
to force to adiv
to 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