-
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
[New Nav Feature] Move collapsible nav toggle button to be part of EuiCollapsibleNav #3168
[New Nav Feature] Move collapsible nav toggle button to be part of EuiCollapsibleNav #3168
Conversation
Preview documentation changes for this PR: https://eui.elastic.co/pr_3168/ |
Do you still want to make this change then? |
Yes because at the very least it allows us to control the aria-labels for the toggle button and anything else that might be drummed up in implementation. |
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.
LGTM! One small change that's not even directly related to this work...
Preview documentation changes for this PR: https://eui.elastic.co/pr_3168/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_3168/ |
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.
Text looks good, ty!
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.
LGTM!
Co-Authored-By: Greg Thompson <thompson.glowe@gmail.com>
Preview documentation changes for this PR: https://eui.elastic.co/pr_3168/ |
…iCollapsibleNav (#3168) * Move collapsible nav toggle button to be part of EuiCollapsibleNav * No role group * Alterations to top links and added `pinnable` prop to pinnable list items
* [Feature] Added `EuiCollapsibleNav` component (#2977) * [New Nav Feature] Added `ghost` colored EuiListGroupItem (#3018) * [New Nav Feature] Created `EuiCollapsibleGroup` (#3031) * [New Nav Feature] EuiPinnableListGroup (#3061) * [K8 Nav Feature] Added `home` and `menu` glyphs to EuiIcon (#3109) * [New Nav Feature] Final docs examples and patterns (#3117) * [New Nav Feature] Move collapsible nav toggle button to be part of EuiCollapsibleNav (#3168)
* [Feature] Added `EuiCollapsibleNav` component (elastic#2977) * [New Nav Feature] Added `ghost` colored EuiListGroupItem (elastic#3018) * [New Nav Feature] Created `EuiCollapsibleGroup` (elastic#3031) * [New Nav Feature] EuiPinnableListGroup (elastic#3061) * [K8 Nav Feature] Added `home` and `menu` glyphs to EuiIcon (elastic#3109) * [New Nav Feature] Final docs examples and patterns (elastic#3117) * [New Nav Feature] Move collapsible nav toggle button to be part of EuiCollapsibleNav (elastic#3168)
Better for accessibility
Instead of forcing the consumer to remember all the
aria-
bits that the toggle button needs, the EuiCollapsibleNav component now behaves similar to the EuiPopover component where it accepts abutton
andisOpen
props.It can then also handle the visibility of the button when docked internally.
Checklist
[ ] Checked for breaking changes and labeled appropriately[ ] A changelog entry exists and is marked appropriately