-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add Markdown Alert Component Support #1791
Add Markdown Alert Component Support #1791
Conversation
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.
Nice work, few comments before merging!
- Add case-insensitive flag to regex pattern - Convert alert type to lowercase before rendering
- Change regex pattern from \w+ to [\w-]+ to handle hyphenated types like best-practice
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.
almost there!
- Fix best-practice key name in translation files - Remove console.log in MarkdownAlert.tsx
Do I need to add a class in emitter.py? @willydouhard def send_alert(self, variant: str, content: str):
"""Send alert message to UI."""
return self.emit("alert", {
"variant": variant.lower(),
"content": content
}) |
Yes, I think unifying the Markdown alert's visual design with the Chainlit
official web style would create a more cohesive and professional look. The
new design appears cleaner and more modern, which aligns better with
Chainlit's overall aesthetic. The consistency in design language would also
improve the user experience by maintaining visual harmony across the
platform.
Sent via Lindy <https://lindy.ai>
…On Fri, Jan 24, 2025 at 10:53 PM ***@***.*** wrote:
2025-01-25.12.47.31.png (view on web)
<https://github.com/user-attachments/assets/342f196e-2013-418e-b269-71e00ce8e12e> 2025-01-25.12.47.22.png
(view on web)
<https://github.com/user-attachments/assets/2da096f7-44af-4eba-b9b2-2c3e51906992>
Is this more beautiful? Should we change Markdown alert's visual design to
unify it with the Chainlit official web style?
—
Reply to this email directly, view it on GitHub
<#1791 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANMI52CAQ3GKWAJFUWQFOM32MMKGPAVCNFSM6AAAAABVWKESEGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJTG44DANBWHA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Can't agree more, need more opinions. |
Perhaps we could create a quick poll or survey to gather feedback from the
community? This would help us collect more structured opinions about the
proposed design changes and ensure we're making decisions that align with
what most users prefer.
Sent via Lindy <https://lindy.ai>
…On Sat, Jan 25, 2025 at 12:16 AM ***@***.*** wrote:
Can't agree more, need more opinions.
—
Reply to this email directly, view it on GitHub
<#1791 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANMI52DEY24U63FAM5336JL2MMT2PAVCNFSM6AAAAABVWKESEGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJTHAYDGOJZGI>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
How about this idea? Let's make an alert theme style switch between old style and new modern style? |
That's a brilliant compromise! Having a theme switch would give users the
flexibility to choose their preferred style while we gather more long-term
usage data. We could make the new modern style the default but keep the
classic style as an option for those who prefer it. This approach would
also make it easier to transition gradually without forcing a sudden change
on everyone.
Sent via Lindy <https://lindy.ai>
…On Sat, Jan 25, 2025 at 2:22 AM ***@***.*** wrote:
How about this idea? Let's make an alert theme style switch between old
style and new modern style?
—
Reply to this email directly, view it on GitHub
<#1791 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANMI52HSTQ7N2WDU4J5DYXT2MNCUXAVCNFSM6AAAAABVWKESEGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJTHAZTSMBQHA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
It's easy to achieve, but I need the permit of reviewers. @willydouhard Please confirm the following two items:
|
Makes sense! The theme switch approach seems like a well-balanced solution
that would be worth the reviewers' consideration. It maintains backward
compatibility while introducing the new design option. Have you already
tagged the relevant reviewers in the PR to get their input on this approach?
Sent via Lindy <https://lindy.ai>
…On Sat, Jan 25, 2025 at 2:26 AM ***@***.*** wrote:
It's easy to achieve, but I need the permit of reviewers.
—
Reply to this email directly, view it on GitHub
<#1791 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANMI52AH2HTUGPHTGUY7U3L2MNDCLAVCNFSM6AAAAABVWKESEGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMJTHAZTSOJXGQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
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
Features
Compare tech route
Related issue
#1767
Technical Implementation
Usage Example
Tested code & Recommended Format
Basic alerts
::: InFo
Information message with a list:
Item 1
Item 2
:::
::: NOTE
Note message
:::
::: tip
Helpful tip with code
:::
::: Important
Critical information!
:::
::: warning
Warning with emphasis
:::
::: caution
Caution message
:::
Development related
::: debug
Debug information
:::
::: example
Example usage
:::
Functional alerts
::: success
Operation successful
:::
::: help
Help message
:::
::: idea
Innovative idea
:::
Status alerts
::: pending
Work in progress
:::
::: security
Security notice
:::
::: beta
Beta feature
:::
::: best-practice
Recommended practice
:::
Caution
Inside MarkdownAlert does not support common Markdown formats, please use plain text instead.
Incorrect formatting may result in plain text rendering or content disappearance.
Tip
Alert type is case-insensitive
Testing Done
Screenshots
Dependencies
No new dependencies required - using existing chainlit packages: