-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
refactor(ui): improve first time alerting experience #14917
Conversation
These should be handled by Clockface instead
Not 100% sold on the column orderings but we will see how it feels. i think it's a net positive for launch. |
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 love the little question mark things, I think we should put them all over the UI.
There's some LINEBREAK
/ <br />
weirdness all over. I suspect that it is hacky around some component API that could be better.
return ( | ||
<EmptyState size={ComponentSize.Small} className="alert-column--empty"> | ||
<EmptyState.Text | ||
text="A Check is a periodic query that the system performs against your time series data that will generate a status" | ||
text="Looks like you have not created a Check yet LINEBREAK LINEBREAK You will need one to be notified about LINEBREAK any changes in system status" |
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.
What's up with this LINEBREAK
stuff? Could markdown be passed instead? Or the text passed as multiple props?
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.
Glad you asked! The empty state component is overly strict and ends up shooting itself in the foot in the process. I am planning a small change in the next clockface update that should remove all this funk
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.
Ah ok, awesome
const tooltipContents = ( | ||
<> | ||
A <strong>Check</strong> is a periodic query that the system | ||
<br /> |
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.
Do these <br />
have to be here? Feels like an indication that something else could be fixed
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 manually placed the line breaks in a way that makes it nice to read rather than relying on left/right padding
Closes #14911
The goal is to make it more obvious what to do first in order to get set up with the Monitoring & Alerting feature