-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Small improvements to add cluster page #29142
Conversation
💚 Build Succeeded |
Pinging @elastic/es-ui |
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.
This looks great! I tested the UI locally. I spotted a couple things we can improve, but once they're addressed this looks ready to merge.
color="subdued" | ||
content={seedNodeTooltip} | ||
/> | ||
</EuiFlexItem> |
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.
We can nest the X icon and "Not connected" text inside another flex group, to get the tooltip to center over the icon like this:
<EuiFlexGroup gutterSize="s" alignItems="center">
<EuiFlexItem grow={false}>
<EuiFlexGroup gutterSize="s" alignItems="center">
<EuiFlexItem grow={false}>
{icon}
</EuiFlexItem>
<EuiFlexItem>
<EuiText>
{message}
</EuiText>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiIconTip
color="subdued"
content={seedNodeTooltip}
/>
</EuiFlexItem>
</EuiFlexGroup>
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.
@@ -54,6 +59,13 @@ export function ConnectionStatus({ isConnected }) { | |||
{message} | |||
</EuiText> | |||
</EuiFlexItem> | |||
<EuiFlexItem> | |||
<EuiIconTip | |||
type="iInCircle" |
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.
iInCircle
is the default value for type
, so we can remove this.
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!
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
💚 Build Succeeded |
Improve text around what port should be entered, and add a tooltip that explains the "Not Connected" error message.
Summary
Fixes #28514. Adds two clarifying points to the add cluster page:
Checklist
This was checked for cross-browser compatibility, including a check against IE11Documentation was added for features that require explanation or tutorialsUnit or functional tests were updated or added to match the most common scenariosThis was checked for keyboard-only and screenreader accessibilityFor maintainers