Skip to content

feat: Add styles to make Markdown work#3485

Merged
skynetigor merged 8 commits intokeephq:mainfrom
hp77-creator:feat_3353
Feb 18, 2025
Merged

feat: Add styles to make Markdown work#3485
skynetigor merged 8 commits intokeephq:mainfrom
hp77-creator:feat_3353

Conversation

@hp77-creator
Copy link
Copy Markdown
Contributor

@hp77-creator hp77-creator commented Feb 16, 2025

Closes #3353
/claim #3353

📑 Description

Modified styles in markdown to use the styles with which the incidents are created:

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

ℹ Additional Information

This is how it looks now
image

@vercel
Copy link
Copy Markdown

vercel bot commented Feb 16, 2025

@hp77-creator is attempting to deploy a commit to the KeepHQ Team on Vercel.

A member of the Team first needs to authorize it.

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. Feature A new feature UI User interface related issues labels Feb 16, 2025
@hp77-creator
Copy link
Copy Markdown
Contributor Author

@talboren Please let me know if this is alright?

@skynetigor
Copy link
Copy Markdown
Contributor

skynetigor commented Feb 16, 2025

@hp77-creator Well done!
I've just checked out your branch and tested locally!
I used Incident chat to generate summary and all formatting was preserved in Incident summary.
Looks great 👍
image

However, as far as I see, there is some incompatibility with Summary form control in "Edit incident" modal. This is the screenshot:
image

@hp77-creator
Copy link
Copy Markdown
Contributor Author

That shouldn't have happened, I think it is not rendering it, let me look into it. Will update you. Thanks

@hp77-creator
Copy link
Copy Markdown
Contributor Author

@skynetigor
can you tell me how did you get this screen?
I clicked on Edit incidents and it showed something like this:
image

@hp77-creator
Copy link
Copy Markdown
Contributor Author

@skynetigor can you share the type of formatting that you have used in your incident, I will try to recreate it.

@skynetigor
Copy link
Copy Markdown
Contributor

skynetigor commented Feb 16, 2025

@skynetigor can you tell me how did you get this screen? I clicked on Edit incidents and it showed something like this: image

You need to have AI enabled and try to generate Incident summary with AI producing formatted text. It then is displayed good on Incident, but when you try to edit the incident, in the rich text Summary editor the formatting is broken..
https://docs.keephq.dev/deployment/configuration#openapi

@hp77-creator
Copy link
Copy Markdown
Contributor Author

Understood, but I don't have any API_KEY, so I guess I can't test it that way, can you share the text, which you see in the Edit window, I can try with that manner

@skynetigor
Copy link
Copy Markdown
Contributor

`Incident Title: Cloudwatch Incident

Summary of Alerts:

  1. ServiceMemoryAlert: Service memory usage is critical in 'worker-service'.
  2. ContainerMemoryHigh: Container in 'cache-service' using excessive memory.
  3. EndpointFailures: Critical endpoint failure detected in 'order-system', Priority: P1.
  4. HighCPUUsage: CPU utilization is above threshold in 'mailing-app'.
  5. PodRecycled: Pod recycled due to resource constraints in 'main-app'.
  6. APIErrorRate: API error rate above normal levels in 'payment-api', Priority: P1.
  7. HighMemoryUsage: Memory utilization exceeded threshold in 'api-service'.
  8. ServiceErrors: High error count in 'user-service', Priority: P1.
  9. HighCPUUsageOnAPod: Pod CPU usage exceeds safe limits in 'producers'.

Severity of all incidents: Info
Priority varies from P1 to P3.`

This is the summary generated by OpenAI and broken in rich text editor.
The main issue I think is because it's markdown but rich text editor works with plain html..
image
image

@skynetigor
Copy link
Copy Markdown
Contributor

Sorry, I'm struggling to put this markdown text here because GH interprets it 🥲
I'll find a way to attach it

@hp77-creator
Copy link
Copy Markdown
Contributor Author

If you can add it in a txt file and attach it, i guess that will work 🤔

@skynetigor
Copy link
Copy Markdown
Contributor

incident summary.txt

@talboren talboren mentioned this pull request Feb 17, 2025
4 tasks
Signed-off-by: hp <himanshu.dn.pandey@gmail.com>
@hp77-creator
Copy link
Copy Markdown
Contributor Author

@skynetigor yes you are right, so showing it in ReactQuill will need some more changes inside editor, just want to know was it working before? In the scope of original issue we wanted to get only format for summary, right? should I update ReactQuill component?

@skynetigor
Copy link
Copy Markdown
Contributor

Hi @hp77-creator
I noticed that "whitespace-pre-wrap" makes summary with markdown look ugly. If I remove this class, it looks better.
Could you please check this?
With the class:
image
Without the class:
image
This is the summary I tested with:
incident summary.txt

Let's check this issue (if it's an issue) and merge it. Although not all cases covered, I think the main issue was solved here. Now "summary" on Incident is displayed well for both AI generated summary and for summary created using rich text editor in create/edit form.

But the issue with incorrect handling of markdown in rich-text editor is worth being an another issue.

@hp77-creator
Copy link
Copy Markdown
Contributor Author

hp77-creator commented Feb 18, 2025

Noted @skynetigor
It was giving merge conflict, so I simply added it to resolve the conflict, will remove whitespace-pre-wrap.

I agree that create/edit one could be another issue. I am willing to take up that.

@hp77-creator
Copy link
Copy Markdown
Contributor Author

@skynetigor I have removed it. Let me know on the further course of action.

Copy link
Copy Markdown
Contributor

@skynetigor skynetigor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@skynetigor
Copy link
Copy Markdown
Contributor

@hp77-creator merging it, thank you for your contribution 🙏

@skynetigor skynetigor merged commit e823b1d into keephq:main Feb 18, 2025
@github-actions
Copy link
Copy Markdown
Contributor

🚂 Fantastic work @hp77-creator! Your very first PR to keep has been merged! 🎉🥳

You've just taken your first step into open-source, and we couldn't be happier to have you onboard. 🙌
If you're feeling adventurous, why not dive into another issue and keep contributing? The community would love to see more from you! 🚀

For any support, feel free to reach out on the community: https://slack.keephq.dev. Happy coding! 👩‍💻👨‍💻

@hp77-creator
Copy link
Copy Markdown
Contributor Author

@skynetigor thanks a lot for being so patient and helpful. I will pick up the other issue as well. Let me know when you create it. Also joining slack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🙋 Bounty claim Feature A new feature size:XS This PR changes 0-9 lines, ignoring generated files. UI User interface related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[➕ Feature]: Support formatted text in Incident summary

3 participants