Add component formatting support with Table class - #113
Conversation
There was a problem hiding this comment.
It's mostly good for me, the only thing I don't like is the html formatting. While we don't use anything else; write raw html, I think it's worth doing something like https://github.com/Code-Society-Lab/ada/blob/bc56c97b129e8c53d63cd1418f520caa0f802d21/bot/extensions/moderation/kick_extension.py#L10
You also forgot tests
|
main's history has to be rewritten to adjust a commit's message, no file content changed but your PR's diff may look off until you either click "Update branch" or rebase locally: |
1329faf to
2b76240
Compare
PenguinBoi12
left a comment
There was a problem hiding this comment.
Just one thing that's kind of important but the rest lgtm!
| table title are HTML-escaped when rendered. | ||
| """ | ||
|
|
||
| def __init__(self, *, title: str, columns: int = 2) -> None: |
There was a problem hiding this comment.
I think we should add a validation on columns to in case someone puts 0 or a negative amount of columns:
if columns < 1:
raise ValueError(...)nit: I think we should call that column_count or something. When I see columns I expect a list of columns.
|
|
||
| @bot.command() | ||
| async def weather(ctx): | ||
| weather = Table(title="Los Angeles", columns=2) |
There was a problem hiding this comment.
nit: I would remove columns=2 since it's the default value.
Description
Added a new Table component for structured message rendering and support for sending component-formatted messages in Room and Context. This will improve the messaging capabilities by allowing more organized and visually appealing message formats.
Type of Change
Pre-merge Checklist
pytestmypyblack .