-
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
[EuiHeader] Added sections
and position
props
#2928
Conversation
Preview documentation changes for this PR: https://eui.elastic.co/pr_2928/ |
Hmm, don't review this PR just yet, I've got a truncation issue with the breadcrumbs. |
sections
and position
propssections
and position
props
I take this one back for now, I want to rethink it some more. This is originally just the quick way I did it in the K8 POC, but I don't think it's the right solution. Since I can't put into draft status, it is in WIP status. |
sections
and position
propssections
and position
props
Ok, I think I came up with a better solution and I've pulled this back into read for review status. |
Preview documentation changes for this PR: https://eui.elastic.co/pr_2928/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_2928/ |
I really need to start making sure I push my changes before I create or update PR's. I'm really sorrry @thompsongl & @snide if you've already starting looking at this, but apparently I never pushed my last commit which actually fixes it from my last attempt. It is now up to date. 😑 |
Preview documentation changes for this PR: https://eui.elastic.co/pr_2928/ |
jenkins, test this |
Preview documentation changes for this PR: https://eui.elastic.co/pr_2928/ |
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.
Code changes LGTM
Need to rebase or merge master and update the CL entry
Preview documentation changes for this PR: https://eui.elastic.co/pr_2928/ |
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.
I reviewed the code and the build preview and everything LGTM! 🎉
Preview documentation changes for this PR: https://eui.elastic.co/pr_2928/ |
Continuing support for new nav updates
Sections
One of the most complex components to build is the EuiHeader. There are 6 different components you have to weave together the right way to display in the header.
This PR is an attempt to simplify a portion of that by allowing the user to pass
sections
instead ofchildren
.The docs example basically spits out the same content as the example above it but in 20 less lines of code (where it matters: in the render).
The passed prop looks something like this:
Where
items
is just an array of nodes, andbreadcrumbs
is an array of breadcrumbs. All the truncation should work properly and each section is spaced viaspace-between
.Position
I also decided that we should be managing the fixed position since we also know the height of the header. So I added the prop
position
which defaults tostatic
but if changed tofixed
will also apply a class to the window body with the correct padding.Checklist
[ ] Checked for breaking changes and labeled appropriately[ ] Checked for accessibility including keyboard-only and screenreader modes