-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
chat: add a mode specific class to the messages #1812
Conversation
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
---|---|---|---|
🟢 | Statements | 87.68% (+0.01% 🔼) |
3196/3645 |
🟡 | Branches | 69.23% (+0.04% 🔼) |
2200/3178 |
🟢 | Functions | 83% (+0.02% 🔼) |
586/706 |
🟢 | Lines | 88.22% (+0.01% 🔼) |
2958/3353 |
Test suite run success
647 tests passing in 43 suites.
Report generated by 🧪jest coverage report action from ca614ab
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
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.
Let's talk before I approve/reject
<Paper | ||
sx={sender ? senderPaperSx : otherPaperSx} | ||
data-idx={index} | ||
className={getSuffixedClassNames(className, "-" + mode)} |
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 would have called these classes
taipy-chat-<mode>-[incoming|outgoing]-message
and probably add
taipy-chat-[incoming|outgoing]-message
as well
and maybe add
taipy-chat-message
What say you?
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.
there's a class earlier in the hierarchy taipy-chat-received
taipy-chat-sent
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.
is that enough for you ? @FabienLelaquais
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.
It is!
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.
👌
<Paper | ||
sx={sender ? senderPaperSx : otherPaperSx} | ||
data-idx={index} | ||
className={getSuffixedClassNames(className, "-" + mode)} |
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.
It is!
resolves #1807