-
Notifications
You must be signed in to change notification settings - Fork 863
Set min-width on header section items only when they are not empty #6158
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
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6158/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6158/ |
|
|
||
| @include euiBreakpoint('xs') { | ||
| .euiHeaderSectionItem { | ||
| .euiHeaderSectionItem:not(:empty) { |
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 wonder if we should check instead in EuiHeaderSectionItem if the component has children, if not it doesn't render at all.
// In "/src/components/header/header_section/header_section_item.tsx" instead of returning:
return (
<div className={classes} {...rest}>
{children}
</div>
);
// we check if there is a children and if is not, we don't render anything:
return children ? (
<div className={classes} {...rest}>
{children}
</div>
) : null;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.
+1 to returning null when children isn't present, this will also avoid applying the border styling when no content is present.
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.
Thanks a lot, @miukimiu! I agree, much better handling of the empty state. Let me apply your suggestion to this PR.
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6158/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6158/ |
elizabetdev
left a comment
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.
Thanks, @yuliacech. LGTM! 🎉
I just have one minor suggestion for the CL.
Co-authored-by: Elizabet Oliveira <[email protected]>
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_6158/ |
Summary
While working on the guided onboarding project, I noticed that when a header section item is empty, it's still rendered with a
min-widthsetting. Instead we only render this component, if it's not empty.Screenshot before
Checklist