-
-
Notifications
You must be signed in to change notification settings - Fork 830
Render image data in reactions #7903
Render image data in reactions #7903
Conversation
Many messaging services such as Slack and Discord allow for custom images for reacts, rather than only standard emoji. Currently Element will only show the plain text of the reaction key. This PR updates element to render the media content from reaction events. This does not add a way for users to add custom reactions yet. This can be useful in the case of bots and bridges, which will be able to immediately use this functinality. A picker for Element users will be added in the future.
3d87770
to
fe674e8
Compare
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.
Thank you for taking a look at this!
Code-wise this is largely okay, though I do somewhat question the approach. Custom emoji also extends into non-reaction messages which can affect how the technical side gets created.
Before it goes for design/product review, I think it'd be best to involve the Matrix Spec process to align a technical solution. There's already some proposals out there for this sort of thing, so it may just be a case of implementing one of them.
The larger context with custom emoji (and stickerpacks) in Matrix is that it's currently in desperate need of review and technical design. #matrix-spec:matrix.org on Matrix can provide more detail about both custom emoji and the proposal process.
@@ -102,7 +102,7 @@ function mightContainEmoji(str: string): boolean { | |||
*/ | |||
export function unicodeToShortcode(char: string): string { | |||
const shortcodes = getEmojiFromUnicode(char)?.shortcodes; | |||
return shortcodes?.length ? `:${shortcodes[0]}:` : ''; | |||
return shortcodes?.length ? `:${shortcodes[0]}:` : char; |
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.
unintentional change?
@@ -0,0 +1,64 @@ | |||
/* | |||
Copyright 2018 New Vector Ltd |
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.
This should probably be your copyright.
This change doesn't conflict and is compatible with either matrix-org/matrix-spec-proposals#1951 or matrix-org/matrix-spec-proposals#2545, as both those deal mainly with distributing packs of images and don't cover reactions at all. I can still put together an msc for this since it might help to make this explicit |
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.
Review was re-requested, however I feel this is blocked on product, spec, and design (in roughly that order, but spec is the more important problem at the moment).
would it be possible to put this and #8087 behind a labs flag or something so that the people who are itching for this can enable, with the understanding that the work is still being finalized from the spec and product side? i would love to be able to share with my users that the feature they've been asking for for the last 2+ years is at least making progress, even if it's preliminary. using labs flags seems like a pretty common use case for early implementation of non-finalized spec feature testing in element. |
Thank you for your contribution. @daniellekirkwood and I synced up today and have decided to proceed with #11087. I'm closing this pull request because it largely achieves the same thing but has the downside of using the short code as key which has already been criticized in https://github.com/matrix-org/matrix-spec-proposals/pull/3746/files#r866285147. |
Many messaging services such as Slack and Discord allow for custom
images for reacts, rather than only standard emoji. Currently Element
will only show the plain text of the reaction key.
This PR updates element to render the media content from reaction
events. This does not add a way for users to add custom reactions yet.
This can be useful in the case of bots and bridges, which will be able
to immediately use this functionality. A picker for Element users will
be added in the future.
This is a solution to element-hq/element-meta#339
Similarly to stickers, the event type should simply be sent with an image content, and it will be displayed appropriately. For example
Signed-off-by: Andrew Ryan [email protected]
Original behavior / fallback:
Updated to display image:
Here's what your changelog entry will look like:
✨ Features