Skip to content
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

Support attachments with title, test and author #18

Merged
merged 27 commits into from
Sep 4, 2023

Conversation

psrpinto
Copy link
Member

@psrpinto psrpinto commented Aug 30, 2023

This PR improves rendering of slack attachments. Not all fields are supported (e.g. images), but the most common ones should be.

This PR also includes some further refactors, and also fixes an issue that caused edits in threads to be posted as replies (upstream issue).

Additionally, we're now using a markdown parser, which simplifies a lot of things (e.g. don't need to parse blockquotes ourselves).

Screen captures

On Matrix

Screenshot 2023-09-01 at 17 01 55

On Matrix, before this PR

Screenshot 2023-09-01 at 17 02 56

On Slack

265071459-ecd32878-fb7e-4f5b-b4a6-8397a1116586

@psrpinto psrpinto changed the base branch from develop to message-attachments August 30, 2023 15:33
@psrpinto psrpinto force-pushed the message-attachments-2 branch from da8eac9 to 34c159b Compare September 1, 2023 10:44
@psrpinto psrpinto changed the base branch from message-attachments to develop September 1, 2023 10:44
@psrpinto psrpinto marked this pull request as ready for review September 1, 2023 16:24
@psrpinto psrpinto requested a review from ashfame September 1, 2023 16:24
@akirk
Copy link
Member

akirk commented Sep 4, 2023

Just the failing test is prevening this from being merged, @psrpinto is working on fixing it.

@akirk
Copy link
Member

akirk commented Sep 4, 2023

An interesting sideeffect of this is when you write Markdown on Slack, it will render as formatted on Matrix when bridged. (You can press cmd-shift-f in Slack to achieve formatting there)

Remark: attachments in Slack are deprecated and the Block Kit should be used, we still need to add support for that.

@akirk
Copy link
Member

akirk commented Sep 4, 2023

Wow, case-insensitive vs case-sensitive filesystem!

@psrpinto
Copy link
Member Author

psrpinto commented Sep 4, 2023

Yep, noticed it by chance, turns out it was the issue.

Merging.

@psrpinto psrpinto merged commit ea107f4 into develop Sep 4, 2023
@psrpinto psrpinto deleted the message-attachments-2 branch September 4, 2023 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants