-
Notifications
You must be signed in to change notification settings - Fork 843
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
[Feature] Converting EuiPageTemplate to use ReactContext instead of cloneChildren #5987
[Feature] Converting EuiPageTemplate to use ReactContext instead of cloneChildren #5987
Conversation
Preview documentation changes for this PR: https://eui.elastic.co/pr_5987/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5987/ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5987/ |
@thompsongl This PR is good now, do you mind doing a quick once-over? |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5987/ |
Nevermind, sending this back to draft because the portal doesn't work when nested... 😖 |
Ok, ready for review again 🤦♀️ |
Preview documentation changes for this PR: https://eui.elastic.co/pr_5987/ |
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.
Nice! This turned out well; LGTM
This reverts commit e50dbc6.
Preview documentation changes for this PR: https://eui.elastic.co/pr_5987/ |
Flakey? |
CI's drunk, I need to move on, since this is merging into a feature branch anyway, I'm merging. |
Summary
Since most of Kibana's usages wildly separate the content from the template, the cloning method was not going to work as it required the children to be direct descendents without any file obfuscation.
So instead, I'm creating a template context in order to pass through the top-level props like padding and restricted width.
The exception of which is the sidebar. This needs to be a direct child because the presence of this exact element determines the configuration. I've noted this in the docs.
The bottom bar now uses
createPortal
to insert itself at the right place.Checklist
[ ] Checked in both light and dark modes[ ] Checked in mobile[ ] Checked Code Sandbox works for any docs examples[ ] Checked for breaking changes and labeled appropriately[ ] Checked for accessibility including keyboard-only and screenreader modes[ ] Updated the Figma library counterpart[ ] A changelog entry exists and is marked appropriately