-
Notifications
You must be signed in to change notification settings - Fork 43
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
🐛 Show first non-empty line in incident message overflow tab #1841
Conversation
Signed-off-by: Ian Bolton <ibolton@redhat.com>
|
||
const getFirstNonEmptyLine = (message: string) => { | ||
const lines = message.split("\n"); | ||
return lines.find((line) => line.trim() !== "") || "No content available."; |
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.
IMHO the no-content label should be styled as a placeholder to clearly distinguish from the analyzed code. If not possible I would use empty string.
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.
Since this text is parsed as Markdown, there are some edge cases that could be a problem. If the first non-empty line is triple back-tick (like for a code block), weird things could happen.
I don't mind if there is markdown message here like _(empty message)_
or some such, but it would make more sense up in the <Td>
.
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 didn't not realize some stuff could be markdown! Craziness
|
||
const getFirstNonEmptyLine = (message: string) => { | ||
const lines = message.split("\n"); | ||
return lines.find((line) => line.trim() !== "") || "No content available."; |
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.
Since this text is parsed as Markdown, there are some edge cases that could be a problem. If the first non-empty line is triple back-tick (like for a code block), weird things could happen.
I don't mind if there is markdown message here like _(empty message)_
or some such, but it would make more sense up in the <Td>
.
<ReactMarkdown components={markdownPFComponents}> | ||
{`${incident.message.split("\n")[0]} ...`} | ||
{`${getFirstNonEmptyLine(incident.message)} ...`} | ||
</ReactMarkdown> |
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.
Maybe more like "if message is blank (i.e. only has whitespace, is empty string, or is falsey), then render a "no message" placeholder else render the ReactMarkdown
component.
Then getFirstNonEmptyLine()
can be assured there is a line to find!
Signed-off-by: Ian Bolton <ibolton@redhat.com>
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, just a non-blocking nit-pick on the function component
return nonEmptyLine || null; | ||
}; | ||
|
||
const messageDisplayComponent = (message: string) => { |
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.
const messageDisplayComponent = (message: string) => { | |
const MessageDisplayComponent: React.FC<{message: string}> = ({ message }) => { |
<ReactMarkdown components={markdownPFComponents}> | ||
{`${incident.message.split("\n")[0]} ...`} | ||
</ReactMarkdown> | ||
{messageDisplayComponent(incident.message)} |
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.
{messageDisplayComponent(incident.message)} | |
<MessageDisplayComponent message={incident.message} /> |
Resolves https://issues.redhat.com/browse/MTA-1959 --------- Signed-off-by: Ian Bolton <ibolton@redhat.com> Signed-off-by: Cherry Picker <noreply@github.com>
…1851) Resolves https://issues.redhat.com/browse/MTA-1959 --------- Backport-of: #1841 Signed-off-by: Ian Bolton <ibolton@redhat.com> Signed-off-by: Cherry Picker <noreply@github.com> Co-authored-by: Scott Dickerson <sdickers@redhat.com>
Resolves https://issues.redhat.com/browse/MTA-1959